Skip to content
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

Put HTTP headers into context in Jaeger Thrift receiver #2407

Conversation

pavolloffay
Copy link
Member

@pavolloffay pavolloffay commented Jan 26, 2021

Signed-off-by: Pavol Loffay [email protected]

Description:

Put HTTP headers in Jaeger receiver into context object. I need to access haeders in a custom processor to attach tenant ID into span attributes and calculate some metrics.

Also https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/master/processor/routingprocessor uses metadata to access tenant ID header.

Link to tracking Issue:

Resolves #2401

@pavolloffay pavolloffay requested a review from a team January 26, 2021 15:55
@pavolloffay
Copy link
Member Author

@bogdandrutu @tigrannajaryan could you please review?

@codecov
Copy link

codecov bot commented Jan 26, 2021

Codecov Report

Merging #2407 (d61af22) into master (ef40657) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2407   +/-   ##
=======================================
  Coverage   91.91%   91.92%           
=======================================
  Files         270      270           
  Lines       15297    15302    +5     
=======================================
+ Hits        14061    14066    +5     
  Misses        857      857           
  Partials      379      379           
Impacted Files Coverage Δ
receiver/jaegerreceiver/trace_receiver.go 76.07% <100.00%> (+0.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef40657...d61af22. Read the comment docs.

Base automatically changed from master to main January 28, 2021 18:06
@pavolloffay
Copy link
Member Author

FYI in the meantime, we have applied the patch to our fork to unblock us hypertrace/hypertrace-collector#23. It would be great to move this PR to decrease maintenance pain.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Feb 11, 2021
Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

Not sure which headers do you need access to, but there are some information we add related to tenant id (https://github.com/open-telemetry/opentelemetry-collector/blob/main/client/client.go), also there is this discussion #2495

The problem with context is that we drop it in processors like batch.

@pavolloffay
Copy link
Member Author

Not sure which headers do you need access to, but there are some information we add related to tenant id

X-tenant-id is the header name. Our collector is behind proxy that handles auth and injects this header if the access is allowed.

The problem with context is that we drop it in processors like batch.

It's is not an issue for us since we access the headers in the first processor in the pipeline.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2021

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Mar 1, 2021
@pavolloffay
Copy link
Member Author

any update on this @bogdandrutu ?

@github-actions github-actions bot removed the Stale label Mar 2, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2021

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Mar 9, 2021
@github-actions
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Mar 17, 2021
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this pull request Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Access headers in processor
2 participants