opentelemetry-instrumentation-aws-lambda: fix improper handling of header casing#4216
opentelemetry-instrumentation-aws-lambda: fix improper handling of header casing#4216herin049 wants to merge 4 commits intoopen-telemetry:mainfrom
Conversation
|
|
||
| if lambda_event.get("headers"): | ||
| if "User-Agent" in lambda_event["headers"]: | ||
| headers = {k.casefold(): v for k, v in lambda_event["headers"].items()} |
There was a problem hiding this comment.
don't we have any helper to normalize headers in util-http?
There was a problem hiding this comment.
I think we have only calls to lower(), we have an helper to normalize them as attributes, e.g.
def normalise_request_header_name(header: str) -> str:
key = header.lower().replace("-", "_")
return f"http.request.header.{key}"
There was a problem hiding this comment.
Ideally, we can add a CIDict to util-http, but I figured that this is a simple enough fix. Looking through some of the other instrumentation libraries (e.g. Kafka if I recall right), these header casing bugs might be scattered elsewhere. Let me know what you think, I can either add another method to util-http or add a CIDict implementation.
| if not isinstance(headers, dict): | ||
| headers = {} | ||
| return get_global_textmap().extract(headers) | ||
| return get_global_textmap().extract( |
There was a problem hiding this comment.
When I did this in another code base I've only done the header normalization for v1 using this to decide if that was the case:
def should_normalize(event):
request_context = event.get("requestContext", {})
return ("elb" in request_context or "requestId" in request_context) and "http" not in request_context
Maybe here it would be impractical since we are picking more stuff from the headers.
There was a problem hiding this comment.
We'd want to normalize regardless, no? For example, even if you're using API GW v2, it's possible that a caller still might not provide headers as lowercase.
|
|
||
| if lambda_event.get("headers"): | ||
| if "User-Agent" in lambda_event["headers"]: | ||
| headers = {k.casefold(): v for k, v in lambda_event["headers"].items()} |
There was a problem hiding this comment.
I think we have only calls to lower(), we have an helper to normalize them as attributes, e.g.
def normalise_request_header_name(header: str) -> str:
key = header.lower().replace("-", "_")
return f"http.request.header.{key}"
|
I've opted to add a |
3 participants
Description
Fixes issue where request headers are extracted in a case-sensitive manner, resulting in issues with context propagation for API Gateway requests.
Fixes #4106
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.