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.

Sign up for GitHub

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

Jump to bottom

Rename default value of Attribute configuration parameter to Authorization from authorization #8994

Closed
pavankrish123 opened this issue Mar 31, 2022 · 3 comments · Fixed by open-telemetry/opentelemetry-collector#5646
Closed

Rename default value of Attribute configuration parameter to Authorization from authorization #8994

pavankrish123 opened this issue Mar 31, 2022 · 3 comments · Fixed by open-telemetry/opentelemetry-collector#5646
Assignees
@jpkrohling

Comments

@pavankrish123
Copy link
Contributor

Prior Art: open-telemetry/opentelemetry-collector#4982

The client libraries (OAuth2) usually set the authorization tokens as "Authorization: <>". Currently, by default OIDC extension looks for "authorization" header key.
@pavankrish123 pavankrish123 mentioned this issue Mar 31, 2022
Closed
@jpkrohling
Copy link
Member

As mentioned in the other issue, just changing from "A" to "a" would break gRPC. We'd need a way to match this header in a case-insensitive manner.

@pavankrish123
Copy link
Contributor Author

Got it! @jpkrohling I would like to implement this special case insensitivity only for default (Auth and fallback auth). In other cases I am planning to make it case sensitive. Does this sound good?

@pavankrish123 pavankrish123 reopened this Apr 1, 2022
@jpkrohling
Copy link
Member

Does this sound good?

Perhaps. Note that the RFC for the HTTP spec states that headers are case insensitive. So, a user specifying "authorization" would correctly expect we match both "authorization" and "auTHOrization".

Perhaps we should do an optimistic lookup first, and do a case-insensitive search if no values are found?

@jpkrohling jpkrohling mentioned this issue Jul 7, 2022
Closed
@jpkrohling jpkrohling self-assigned this Jul 7, 2022
jpkrohling added a commit to jpkrohling/opentelemetry-collector that referenced this issue Jul 7, 2022
@jpkrohling
Change metadata lookup to be case insensitive
b8d3292 
Before this change, users would make references to headers such as "authorization" and "X-CloudFront-Viewer-Latitude", which would potentially fail, as they would be stored in the context as "X-Cloudfront-Viewer-Latitude" or "Authorization".

This PR changes this behavior, to attempt a case insensitive lookup to the backing map in case the key wasn't found under the requested casing. Under the assumption that the lookup key specified by users won't change, on a subsequent lookup, we proactively copy the value to the looked up key, to make the next lookup faster.

Fixes open-telemetry#5610
Fixes open-telemetry/opentelemetry-collector-contrib#8994

Signed-off-by: Juraci Paixão Kröhling <[email protected]>
@jpkrohling jpkrohling mentioned this issue Jul 7, 2022
Merged
jpkrohling added a commit to jpkrohling/opentelemetry-collector that referenced this issue Jul 8, 2022
@jpkrohling
Change metadata lookup to be case insensitive
f9ee728 
Before this change, users would make references to headers such as "authorization" and "X-CloudFront-Viewer-Latitude", which would potentially fail, as they would be stored in the context as "X-Cloudfront-Viewer-Latitude" or "Authorization".

This PR changes this behavior, to attempt a case insensitive lookup to the backing map in case the key wasn't found under the requested casing. Under the assumption that the lookup key specified by users won't change, on a subsequent lookup, we proactively copy the value to the looked up key, to make the next lookup faster.

Fixes open-telemetry#5610
Fixes open-telemetry/opentelemetry-collector-contrib#8994

Signed-off-by: Juraci Paixão Kröhling <[email protected]>
codeboten pushed a commit to open-telemetry/opentelemetry-collector that referenced this issue Jul 13, 2022
@jpkrohling
Change metadata lookup to be case insensitive (#5646)
6417a3a 
* Change metadata lookup to be case insensitive

Before this change, users would make references to headers such as "authorization" and "X-CloudFront-Viewer-Latitude", which would potentially fail, as they would be stored in the context as "X-Cloudfront-Viewer-Latitude" or "Authorization".

This PR changes this behavior, to attempt a case insensitive lookup to the backing map in case the key wasn't found under the requested casing. Under the assumption that the lookup key specified by users won't change, on a subsequent lookup, we proactively copy the value to the looked up key, to make the next lookup faster.

Fixes #5610
Fixes open-telemetry/opentelemetry-collector-contrib#8994

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

* Add changelog entry

Signed-off-by: Juraci Paixão Kröhling <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jpkrohling jpkrohling

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Change metadata lookup to be case insensitive jpkrohling/opentelemetry-collector
2 participants
@jpkrohling @pavankrish123