-
Notifications
You must be signed in to change notification settings - Fork 487
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
feat(faro/receiver): propagate request metadata to downstream consumers #6515
feat(faro/receiver): propagate request metadata to downstream consumers #6515
Conversation
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.
Doc change is small. Over to Agent team for code review.
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.
Thank you, and apologies for the late review! If it's hard to get the test working, maybe a simpler test will do for now?
This one is still sitting in my ToDos... |
Signed-off-by: hainenber <[email protected]>
Signed-off-by: hainenber <[email protected]>
…nsumer Signed-off-by: hainenber <[email protected]>
Intertwined logic between components makes it harder to trace the RC of data race Signed-off-by: hainenber <[email protected]>
Signed-off-by: hainenber <[email protected]>
Signed-off-by: hainenber <[email protected]>
de7b30d
to
dbc9ffa
Compare
I've resolved the merge conflict. Re: simpler test for the change, most of metadata handling logic comes from internals of OpenTelemetry collectors so a E2E test is pretty much warranted. I think we can skip running data race for this particular test for the time being. |
@hainenber I'm really sorry about how long this (and your other PRs) have been sitting around; we've all been super busy preparing for GrafanaCon. Working on helping getting this merged now :) |
@rfratto no worries, the more GA users from GrafanaCon, the better for me also :D Thanks for checking out my past PRs! |
PR Description
Which issue(s) this PR fixes
Fixes #6503
Notes to the Reviewer
Added a E2E test for the whole flow but afraid that it's plagued with data race 😞 . Hope this can be circumvented by running the test without the race detector.
PR Checklist