Skip to content

Comments

feat: instrument error reporting and tracing for go jobs#4819

Merged
michaelkedar merged 6 commits intogoogle:masterfrom
michaelkedar:🪵⚠️🫥
Feb 17, 2026

Hidden character warning

The head ref may contain hidden characters: "\ud83e\udeb5\u26a0\ufe0f\ud83e\udee5"
Merged

feat: instrument error reporting and tracing for go jobs#4819
michaelkedar merged 6 commits intogoogle:masterfrom
michaelkedar:🪵⚠️🫥

Conversation

@michaelkedar
Copy link
Member

  1. Makes error level (and the hypothetical levels above) logs create a new error in the error reporting dashboard, which should hopefully link to the logs
  2. adds traces to all our go jobs, which should maybe let us use the trace explorer to see how long things take in a fancy waterfall graph (related to Investigate using OpenTelemetry for API trace logging #3547)
  • The exporter will create one big span for the exporter, and sub-spans for each ecosystem
  • The relations will create one big span for the relations, and a sub-span for alias/upstream/related
  • gitter traces each http request that is made to it. We can add propagate the same trace from the worker/importer to correlate requests, which should be fun.

This doesn't affect vulnfeeds (yet), since it uses a duplicate logging thing.
To instrument tracing, you have to propagate the context correctly, and the new logger.[LEVEL]Context() commands.

@michaelkedar
Copy link
Member Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request is a significant step forward in observability for the Go jobs, introducing structured logging, error reporting, and distributed tracing. The implementation is largely well-executed, leveraging slog and OpenTelemetry effectively. I've identified a high-severity issue in the error reporting logic that would hinder error grouping in Google Cloud Error Reporting, and a couple of medium-severity inconsistencies in context initialization that should be addressed for better code uniformity. Once these points are addressed, this will be an excellent addition.

Copy link
Contributor

@another-rex another-rex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

)

// InitGlobalLogger initializes the global slog logger and GCP observability clients.
func InitGlobalLogger(ctx context.Context) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is being ran with once, I don't think it makes sense to ask for context here, especially when all callers right now are using context.Background(). This might confused the caller to call this with their specific request context, which we don't want.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we can be fancy and use init() here, and it'll be automatically initialized by anyone that uses any logger functions. Removing the need to have all those if statements in each function. Or people forgetting to init the logger.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the context from InitGlobalLogger, and changed it slightly.
I don't want to chuck it in init, because I don't want to connect to GCP as a side effect of importing. Also, you're supposed to call/defer logger.Close() to flush any errors/traces, and I don't want that silently being forgotten.

I removed the lazy initialisation of the GCP logger - now if you log without calling InitGlobalLogger it will always use the local logger.

another-rex
another-rex previously approved these changes Feb 17, 2026
res = resource.NewWithAttributes(semconv.SchemaURL, semconv.ServiceNameKey.String(serviceName))
}

sampleRate := 0.05
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment descrbing why this is 0.05,

@michaelkedar michaelkedar merged commit fe55e02 into google:master Feb 17, 2026
19 checks passed
@michaelkedar michaelkedar deleted the 🪵⚠️🫥 branch February 17, 2026 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants