chore: Update logging and send errors to NR (#26)

* chore: Update logging

* chore: Fix env var

* chore: Add trace ids to instrumentation

* test: Force a failure

* Revert "test: Force a failure"

This reverts commit c0b952f.

* chore: Notice errors

* test: Force error to test

* fix: Fix so nrApp is shutdown properly

* revert: Manually revert test of NR notice error

* chore: Cleanup

* chore: Fix gitignore

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>

Author: mvicknr

.github/workflows/test_action.yml

@@ -7,6 +7,8 @@ on:
   push:
     branches:
       - '**'
+  schedule:
+    - cron: '0 1 * * *' # This runs the workflow daily at 1am UTC
 
 jobs:
   setup-test-tag:

.gitignore

@@ -7,6 +7,7 @@ agent-metadata-action
 
 # Go coverage
 coverage.out
+coverage.html
 
 # IDE
 .idea/

cmd/agent-metadata-action/main.go

@@ -36,20 +36,20 @@ var ociHandleUploadsFunc = oci.HandleUploads
 
 // initNewRelic initializes the New Relic application
 // Returns nil if APM_CONTROL_NR_LICENSE_KEY is not set (silent no-op mode)
-func initNewRelic() *newrelic.Application {
+func initNewRelic(ctx context.Context) *newrelic.Application {
 	licenseKey := config.GetNRAgentLicenseKey()
 	if licenseKey == "" {
-		_, _ = fmt.Fprintln(os.Stderr, "::warn::Failed to init New Relic - missing license key")
+		logging.Warn(ctx, "Failed to init New Relic - missing license key")
 		return nil
 	}
 
 	// Hardcode staging environment
 	err := config.SetNRAgentHost()
 	if err != nil {
-		_, _ = fmt.Fprintf(os.Stderr, "::warn::Failed to init New Relic, missing host: %v\n", err)
+		logging.Warnf(ctx, "Failed to init New Relic, missing host: %v", err)
 		return nil
 	}
-	fmt.Println("::notice::Using New Relic staging environment")
+	logging.Notice(ctx, "Using New Relic staging environment")
 
 	app, err := newrelic.NewApplication(
 		newrelic.ConfigAppName("agent-metadata-action"),
@@ -64,34 +64,43 @@ func initNewRelic() *newrelic.Application {
 	)
 
 	if err != nil {
-		_, _ = fmt.Fprintf(os.Stderr, "::warn::Failed to init New Relic: %v\n", err)
+		logging.Warnf(ctx, "Failed to init New Relic: %v", err)
 		return nil
 	}
 
-	fmt.Println("::notice::New Relic APM enabled - waiting for connection...")
+	logging.Notice(ctx, "New Relic APM enabled - waiting for connection...")
 
 	// Wait for the app to connect (max 10 seconds)
 	if err := app.WaitForConnection(10 * time.Second); err != nil {
-		fmt.Printf("::warn::New Relic connection timeout: %v - will try to send data anyway\n", err)
+		logging.Warnf(ctx, "New Relic connection timeout: %v - will try to send data anyway", err)
 	} else {
-		fmt.Println("::notice::New Relic connected successfully")
+		logging.Notice(ctx, "New Relic connected successfully")
 	}
 
 	return app
 }
 
 func main() {
-	nrApp := initNewRelic()
+	// Create base context for early logging
+	ctx := context.Background()
 
-	if err := run(nrApp); err != nil {
-		_, _ = fmt.Fprintf(os.Stderr, "::error::%v\n", err)
-		os.Exit(1)
-	}
+	nrApp := initNewRelic(ctx)
+
+	// Run the action
+	err := run(nrApp)
 
+	// Ensure New Relic shuts down gracefully (even on error)
+	// Must happen BEFORE os.Exit() since os.Exit bypasses defers
 	if nrApp != nil {
-		fmt.Println("::notice::Shutting down New Relic - waiting up to 15 seconds to send data...")
+		logging.Notice(ctx, "Shutting down New Relic - waiting up to 15 seconds to send data...")
 		nrApp.Shutdown(15 * time.Second)
-		fmt.Println("::notice::New Relic shutdown complete")
+		logging.Notice(ctx, "New Relic shutdown complete")
+	}
+
+	// Exit with appropriate code
+	if err != nil {
+		logging.Errorf(ctx, "%v", err)
+		os.Exit(1)
 	}
 }
 
@@ -134,16 +143,31 @@ func run(nrApp *newrelic.Application) error {
 func validateEnvironment(ctx context.Context) (workspace string, token string, err error) {
 	workspace = config.GetWorkspace()
 	if workspace == "" {
-		return "", "", fmt.Errorf("GITHUB_WORKSPACE is required but not set")
+		err := fmt.Errorf("GITHUB_WORKSPACE is required but not set")
+		logging.NoticeErrorWithCategory(ctx, err, "environment.validation", map[string]interface{}{
+			"error.operation": "validate_workspace",
+			"error.field":     "GITHUB_WORKSPACE",
+		})
+		return "", "", err
 	}
 
 	if _, err := os.Stat(workspace); err != nil {
-		return "", "", fmt.Errorf("workspace directory does not exist: %s", workspace)
+		noticeErr := fmt.Errorf("workspace directory does not exist: %s", workspace)
+		logging.NoticeErrorWithCategory(ctx, noticeErr, "environment.validation", map[string]interface{}{
+			"error.operation": "validate_workspace",
+			"workspace.path":  workspace,
+		})
+		return "", "", noticeErr
 	}
 
 	token = config.GetToken()
 	if token == "" {
-		return "", "", fmt.Errorf("NEWRELIC_TOKEN is required but not set")
+		err := fmt.Errorf("NEWRELIC_TOKEN is required but not set")
+		logging.NoticeErrorWithCategory(ctx, err, "environment.validation", map[string]interface{}{
+			"error.operation": "validate_token",
+			"error.field":     "NEWRELIC_TOKEN",
+		})
+		return "", "", err
 	}
 
 	logging.Notice(ctx, "Environment validated successfully")
@@ -152,7 +176,7 @@ func validateEnvironment(ctx context.Context) (workspace string, token string, e
 
 // runAgentFlow handles the agent repository workflow
 func runAgentFlow(ctx context.Context, client metadataClient, workspace, agentType, agentVersion string) error {
-	logging.Debug(ctx, "Running agent repository flow")
+	logging.Debugf(ctx, "Running agent repository flow for %s version %s", agentType, agentVersion)
 
 	// Check for .fleetControl directory
 	fleetControlPath := filepath.Join(workspace, config.GetRootFolderForAgentRepo())
@@ -161,15 +185,28 @@ func runAgentFlow(ctx context.Context, client metadataClient, workspace, agentTy
 	}
 
 	// Load configuration definitions (required)
-	configs, err := loader.ReadConfigurationDefinitions(workspace)
+	configs, err := loader.ReadConfigurationDefinitions(ctx, workspace)
 	if err != nil {
+		logging.NoticeErrorWithCategory(ctx, err, "configuration.load", map[string]interface{}{
+			"error.operation": "load_configuration_definitions",
+			"agent.type":      agentType,
+			"agent.version":   agentVersion,
+			"workflow.type":   "agent",
+		})
 		return fmt.Errorf("failed to read configuration definitions: %w", err)
 	}
 	logging.Noticef(ctx, "Loaded %d configuration definitions", len(configs))
 
 	// Load agent control definitions (optional)
-	agentControl, err := loader.ReadAgentControlDefinitions(workspace)
+	agentControl, err := loader.ReadAgentControlDefinitions(ctx, workspace)
 	if err != nil {
+		logging.NoticeErrorWithCategory(ctx, err, "configuration.load", map[string]interface{}{
+			"error.operation": "load_agent_control_definitions",
+			"agent.type":      agentType,
+			"agent.version":   agentVersion,
+			"workflow.type":   "agent",
+			"error.severity":  "warning", // Graceful error
+		})
 		logging.Warnf(ctx, "Unable to load agent control definitions: %v - continuing without them", err)
 		agentControl = nil
 	} else {
@@ -183,21 +220,38 @@ func runAgentFlow(ctx context.Context, client metadataClient, workspace, agentTy
 		AgentControlDefinitions:  agentControl,
 	}
 
-	printJSON("Agent Metadata", metadata)
+	printJSON(ctx, "Agent Metadata", metadata)
 
 	// Step 1: Send to metadata service
 	if err := client.SendMetadata(ctx, agentType, agentVersion, &metadata); err != nil {
+		logging.NoticeErrorWithCategory(ctx, err, "metadata.send", map[string]interface{}{
+			"error.operation": "send_metadata",
+			"agent.type":      agentType,
+			"agent.version":   agentVersion,
+			"workflow.type":   "agent",
+		})
 		return fmt.Errorf("failed to send metadata for %s: %w", agentType, err)
 	}
 
 	ociConfig, err := oci.LoadConfig()
 	if err != nil {
+		logging.NoticeErrorWithCategory(ctx, err, "oci.configuration", map[string]interface{}{
+			"error.operation": "load_oci_config",
+			"agent.type":      agentType,
+			"agent.version":   agentVersion,
+		})
 		return fmt.Errorf("error loading OCI config: %w", err)
 	}
 
 	// Step 2: Upload binaries
-	uploadResults, err := ociHandleUploadsFunc(&ociConfig, workspace, agentType, agentVersion)
+	uploadResults, err := ociHandleUploadsFunc(ctx, &ociConfig, workspace, agentType, agentVersion)
 	if err != nil {
+		logging.NoticeErrorWithCategory(ctx, err, "oci.upload", map[string]interface{}{
+			"error.operation": "upload_binaries",
+			"oci.registry":    ociConfig.Registry,
+			"agent.type":      agentType,
+			"agent.version":   agentVersion,
+		})
 		return fmt.Errorf("binary upload failed: %w", err)
 	}
 
@@ -225,7 +279,7 @@ func runAgentFlow(ctx context.Context, client metadataClient, workspace, agentTy
 				return fmt.Errorf("NEWRELIC_TOKEN is required for artifact signing")
 			}
 
-			if err := sign.SignArtifacts(successfulUploads, ociConfig.Registry, token, repoName, agentVersion); err != nil {
+			if err := sign.SignArtifacts(ctx, successfulUploads, ociConfig.Registry, token, repoName, agentVersion); err != nil {
 				return fmt.Errorf("artifact signing failed: %w", err)
 			}
 		} else {
@@ -242,8 +296,12 @@ func runDocsFlow(ctx context.Context, client metadataClient) error {
 	logging.Debug(ctx, "Running documentation flow")
 
 	// Load metadata from changed MDX files
-	metadataList, err := loader.LoadMetadataForDocs()
+	metadataList, err := loader.LoadMetadataForDocs(ctx)
 	if err != nil {
+		logging.NoticeErrorWithCategory(ctx, err, "docs.load", map[string]interface{}{
+			"error.operation": "load_metadata_from_docs",
+			"workflow.type":   "docs",
+		})
 		return fmt.Errorf("failed to load metadata from docs: %w", err)
 	}
 
@@ -258,7 +316,13 @@ func runDocsFlow(ctx context.Context, client metadataClient) error {
 	successCount := 0
 	for _, entry := range metadataList {
 		if err := sendDocsMetadata(ctx, client, entry); err != nil {
-			logging.Warnf(ctx, "Failed to send metadata for %s: %v", entry.AgentType, err)
+			logging.NoticeErrorWithCategory(ctx, err, "docs.send", map[string]interface{}{
+				"error.operation": "send_docs_metadata",
+				"agent.type":      entry.AgentType,
+				"workflow.type":   "docs",
+				"error.severity":  "warning", // Graceful error
+			})
+			logging.Errorf(ctx, "Failed to send metadata for %s: %v", entry.AgentType, err)
 			continue
 		}
 		successCount++
@@ -276,7 +340,7 @@ func sendDocsMetadata(ctx context.Context, client metadataClient, entry loader.M
 		Metadata: entry.AgentMetadataFromDocs,
 	}
 
-	printJSON(fmt.Sprintf("Docs Metadata (%s %s)", entry.AgentType, version), entry.AgentMetadataFromDocs)
+	printJSON(ctx, fmt.Sprintf("Docs Metadata (%s %s)", entry.AgentType, version), entry.AgentMetadataFromDocs)
 
 	if err := client.SendMetadata(ctx, entry.AgentType, version, &metadata); err != nil {
 		return err
@@ -287,11 +351,11 @@ func sendDocsMetadata(ctx context.Context, client metadataClient, entry loader.M
 }
 
 // printJSON marshals data to JSON and prints it with a debug annotation
-func printJSON(label string, data any) {
+func printJSON(ctx context.Context, label string, data any) {
 	jsonData, err := json.MarshalIndent(data, "", "  ")
 	if err != nil {
-		fmt.Printf("::debug::Failed to marshal %s: %v\n", label, err)
+		logging.Debugf(ctx, "Failed to marshal %s: %v", label, err)
 		return
 	}
-	fmt.Printf("::debug::%s: %s\n", label, string(jsonData))
+	logging.Debugf(ctx, "%s: %s", label, string(jsonData))
 }

cmd/agent-metadata-action/main_test.go

@@ -158,7 +158,7 @@ Release notes content here.
 
 	// Mock GetChangedMDXFiles to return test MDX files
 	originalFunc := github.GetChangedMDXFilesFunc
-	github.GetChangedMDXFilesFunc = func() ([]string, error) {
+	github.GetChangedMDXFilesFunc = func(ctx context.Context) ([]string, error) {
 		return []string{testMDXFile}, nil
 	}
 	defer func() {
@@ -175,11 +175,11 @@ Release notes content here.
 	main()
 
 	outputStr := getStdout()
-	stderrStr := getStderr()
+	_ = getStderr()
 
 	// Verify docs scenario was triggered
 	assert.Contains(t, outputStr, "Running documentation flow")
-	assert.Contains(t, stderrStr, "::notice::Loaded metadata for 1 out of 1 changed MDX files")
+	assert.Contains(t, outputStr, "::notice::Loaded metadata for 1 out of 1 changed MDX files")
 
 	// Verify output contains agent metadata
 	assert.Contains(t, outputStr, "NRJavaAgent")
@@ -331,7 +331,7 @@ func TestRunAgentFlow_SendMetadataFails(t *testing.T) {
 
 func TestRunDocsFlow_LoadMetadataError(t *testing.T) {
 	originalFunc := github.GetChangedMDXFilesFunc
-	github.GetChangedMDXFilesFunc = func() ([]string, error) {
+	github.GetChangedMDXFilesFunc = func(ctx context.Context) ([]string, error) {
 		return nil, assert.AnError
 	}
 	defer func() {
@@ -350,7 +350,7 @@ func TestRunDocsFlow_LoadMetadataError(t *testing.T) {
 
 func TestRunDocsFlow_NoMetadataChanges(t *testing.T) {
 	originalFunc := github.GetChangedMDXFilesFunc
-	github.GetChangedMDXFilesFunc = func() ([]string, error) {
+	github.GetChangedMDXFilesFunc = func(ctx context.Context) ([]string, error) {
 		return []string{}, nil
 	}
 	defer func() {
@@ -399,7 +399,7 @@ version: 1.3.1
 	require.NoError(t, os.WriteFile(testMDXFile2, []byte(mdxContent2), 0644))
 
 	originalFunc := github.GetChangedMDXFilesFunc
-	github.GetChangedMDXFilesFunc = func() ([]string, error) {
+	github.GetChangedMDXFilesFunc = func(ctx context.Context) ([]string, error) {
 		return []string{testMDXFile1, testMDXFile2}, nil
 	}
 	defer func() {
@@ -422,7 +422,7 @@ version: 1.3.1
 	outputStr := getStdout()
 
 	assert.Contains(t, outputStr, "Successfully sent 1 of 2 metadata entries")
-	assert.Contains(t, outputStr, "::warn::Failed to send metadata")
+	assert.Contains(t, outputStr, "::error::Failed to send metadata")
 }
 
 func TestRunAgentFlow_AgentControlDefinitionsError(t *testing.T) {
@@ -587,7 +587,7 @@ func TestRunAgentFlow_SigningSuccess_SingleArtifact(t *testing.T) {
 
 	// Mock OCI handler to return 1 successful upload
 	originalOCIHandler := ociHandleUploadsFunc
-	ociHandleUploadsFunc = func(cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
+	ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
 		return []models.ArtifactUploadResult{
 			createSuccessfulUploadResult("linux-tar", "sha256:abc123", "v1.2.3-linux-amd64"),
 		}, nil
@@ -666,7 +666,7 @@ func TestRunAgentFlow_SigningDisabled_OCINotEnabled(t *testing.T) {
 
 	// Mock OCI handler should not be called since OCI is disabled
 	originalOCIHandler := ociHandleUploadsFunc
-	ociHandleUploadsFunc = func(cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
+	ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
 		return []models.ArtifactUploadResult{}, nil
 	}
 	defer func() { ociHandleUploadsFunc = originalOCIHandler }()
@@ -720,7 +720,7 @@ func TestRunAgentFlow_SigningSkipped_AllUploadsFailed(t *testing.T) {
 
 	// Mock OCI handler to return only failed uploads
 	originalOCIHandler := ociHandleUploadsFunc
-	ociHandleUploadsFunc = func(cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
+	ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
 		return []models.ArtifactUploadResult{
 			createFailedUploadResult("linux-tar"),
 			createFailedUploadResult("windows-zip"),
@@ -779,7 +779,7 @@ func TestRunAgentFlow_SigningError_ServiceFailure(t *testing.T) {
 
 	// Mock OCI handler to return 1 successful upload
 	originalOCIHandler := ociHandleUploadsFunc
-	ociHandleUploadsFunc = func(cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
+	ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
 		return []models.ArtifactUploadResult{
 			createSuccessfulUploadResult("linux-tar", "sha256:abc123", "v1.2.3-linux-amd64"),
 		}, nil