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

Capture custom HTTP headers (Part 2) #681

Merged
merged 6 commits into from
Jan 13, 2025
Merged

Capture custom HTTP headers (Part 2) #681

merged 6 commits into from
Jan 13, 2025

Conversation

GSVarsha
Copy link
Contributor

@GSVarsha GSVarsha commented Jan 2, 2025 •
edited
Loading

Capture custom HTTP headers (Part 2)

What?

aiohttp-client: 🟨 requestHeadersOnExitSpans

aiohttp-server: 🟨 responseHeadersOnEntrySpans

tornado-client: 🟥 requestHeadersOnExitSpans, responseHeadersOnExitSpans

wsgi: 🟨 responseHeadersOnEntrySpans

flask: 🟨 (needs request header testcase)

starlette: 🟨 (needs response header testcase)

Enhancement

Use a single common method to capture custom headers.

@GSVarsha GSVarsha added this to the H1-2025 milestone Jan 2, 2025
@GSVarsha GSVarsha self-assigned this Jan 2, 2025
@GSVarsha GSVarsha added wip work in progress Do not merge Depends on another PR labels Jan 2, 2025
@GSVarsha
test(flask, starlette): cover testcases to capture http headers
e8f9c98 
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
@GSVarsha
aiohttp: capture http headers
19e20cb 
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
@GSVarsha
wsgi: capture response headers
9ebb8ba 
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
@GSVarsha
tornado(client): capture HTTP headers
89221cb 
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
@GSVarsha GSVarsha force-pushed the capture_headers_part2 branch 2 times, most recently from f2086c7 to 0b0fbf6 Compare January 6, 2025 09:01
@GSVarsha GSVarsha marked this pull request as ready for review January 9, 2025 09:10
@GSVarsha GSVarsha requested a review from a team as a code owner January 9, 2025 09:10
@GSVarsha GSVarsha added enhancement Review & Merge and removed wip work in progress Do not merge Depends on another PR labels Jan 9, 2025
@GSVarsha GSVarsha force-pushed the capture_headers_part2 branch from 0b0fbf6 to 7c69cd8 Compare January 9, 2025 09:38
pvital
pvital requested changes Jan 9, 2025
View reviewed changes
Copy link
Member

@pvital pvital left a comment

Choose a reason for hiding this comment

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

Nice refactor @GSVarsha!
I have a few change requests, and I think you should also extend the test_extract_custom_headers in the test_traceutils.py file to cover all the types of headers supported now.

@GSVarsha
enhancement: use a common method to capture custom headers
51c705c 
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
@GSVarsha GSVarsha force-pushed the capture_headers_part2 branch from 7c69cd8 to 51c705c Compare January 10, 2025 10:00
pvital
pvital approved these changes Jan 10, 2025
View reviewed changes
Copy link
Member

@pvital pvital left a comment

Choose a reason for hiding this comment

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

It looks good to me.

@GSVarsha
tests(traceutils): cover all supported header formats
74a0bdb 
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
@GSVarsha GSVarsha requested a review from pvital January 13, 2025 04:44
pvital
pvital approved these changes Jan 13, 2025
View reviewed changes
Copy link
Member

@pvital pvital left a comment

Choose a reason for hiding this comment

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

It looks good to me.

@GSVarsha GSVarsha merged commit 8db8cab into main Jan 13, 2025
14 checks passed
@GSVarsha GSVarsha deleted the capture_headers_part2 branch January 13, 2025 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pvital pvital pvital approved these changes

Assignees

@GSVarsha GSVarsha

Labels
enhancement Review & Merge
Projects
None yet
Milestone
H1-2025
Development

Successfully merging this pull request may close these issues.
2 participants
@GSVarsha @pvital