-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add context propagation checks to component lifecycle tests #13182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add context propagation checks to component lifecycle tests #13182
Conversation
- Introduced context_check_consumer.go to validate context handling - Updated component_test.go.tmpl to inject context in tests - Regenerated component test files across sample components - Cleaned up metadata.yaml files to fix mdatagen errors
@@ -6,7 +6,6 @@ github_project: open-telemetry/opentelemetry-collector | |||
sem_conv_version: 1.9.0 | |||
|
|||
status: | |||
disable_codecov_badge: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep only changes relevant to the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes to metadata.yaml files — including the removal of disable_codecov_badge and events keys were necessary to make mdatagen run successfully.
Without cleaning these fields, mdatagen throws the following error:
Error: failed loading ...metadata.yaml: decoding failed due to the following error(s): 'status' has invalid keys: disable_codecov_badge '' has invalid keys: events
These fields look to be legacy or invalid keys no longer supported by the current mdatagen schema. Cleaning them was essential to regenerate component test files and validate the context propagation logic as part of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clean them in another PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this makes sense. I’ll separate the cleanup of the metadata.yaml files into another PR and keep this one focused on the context propagation validation.
T *testing.T | ||
} | ||
|
||
func (c *ContextCheckLogsConsumer) ConsumeLogs(ctx context.Context, logs plog.Logs) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way of writing tests seems weird, and not really a Go pattern. Why can't we run the assertion inline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback. You're right that the current structure isn’t a typical Go pattern.
I introduced ContextCheck*Consumer
as a lightweight reusable helper to avoid duplicating the same context
assertion logic in each test case. The idea was to keep individual test methods focused on wiring rather than repeating assertions.
That said, I can definitely refactor it to assert inline if preferred. Let me know if you’d like to see it changed.
Description
This change introduces validation logic for context propagation during component lifecycle execution in the OpenTelemetry Collector.
A new helper (context_check_consumer.go) is added to assert context values during Start, Consume, and Shutdown.
The code generation template was updated to inject context.WithValue(...) across all components.
The metadata files were also fixed to pass schema validation for mdatagen.
Link to tracking issue
Fixes #13142
Testing
Documentation