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

Added support of Bearer Auth for HTTP Exporters #5962

Merged
merged 3 commits into from
Oct 29, 2021
Merged

Added support of Bearer Auth for HTTP Exporters #5962

merged 3 commits into from
Oct 29, 2021

Conversation

pavankrish123
Copy link
Contributor

Current implementation for Bearer token auth only supports gRPC clients. This implementation enables the functionality for HTTP clients as well.

Link to tracking Issue: #5927

Testing: : Unit tests and verfied on the mock server Auth headers were set properly.

Documentation: Updated README.md with new path as well.

@pavankrish123
Copy link
Contributor Author

@jpkrohling @anuraaga please review - thank you.

Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

LGTM, but I just realized we don't have support for auth in the confighttp struct like we have for configgrpc, so, we cannot have an HTTP receiver that requires auth.

@pavankrish123
Copy link
Contributor Author

Thanks @jpkrohling - shall I open an issue to handle receivers as well for consistency or we don't want to tackle it?

This pr brings consistency on exporters side though.

BTW, not sure what's happening with k8cluster unit test

@jpkrohling
Copy link
Member

Created #5973 to track the test failure on unrelated code, and tests restarted.

@jpkrohling
Copy link
Member

Another unrelated test failure. Created #5983, tests restarted.

@pavankrish123
Copy link
Contributor Author

Thank you - Looks like tests are fine now.

@jpkrohling jpkrohling added the ready to merge Code review completed; ready to merge by maintainers label Oct 29, 2021
@jpkrohling
Copy link
Member

I don't seem to have merge powers yet, but this is ready to be merged.

@bogdandrutu bogdandrutu merged commit a753475 into open-telemetry:main Oct 29, 2021
@bogdandrutu
Copy link
Member

@jpkrohling fixed the power 👯

@pavankrish123 pavankrish123 deleted the feature/http_bearer branch October 29, 2021 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants