Skip to content

Capture custom HTTP headers (Part 2)#681

Merged
GSVarsha merged 6 commits intomainfrom
capture_headers_part2
Jan 13, 2025
Merged

Capture custom HTTP headers (Part 2)#681
GSVarsha merged 6 commits intomainfrom
capture_headers_part2

Conversation

@GSVarsha
Copy link
Contributor

@GSVarsha GSVarsha commented Jan 2, 2025 •
edited
Loading

Capture custom HTTP headers (Part 2)

What?

This PR adds capturing custom headers feature to the below missing instrumentations:

Instrumentation Feature/ Testcase Status
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 or WIP 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 or WIP 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
2 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 feature Review & Merge

Projects

None yet

Milestone

H1-2025

Development

Successfully merging this pull request may close these issues.

2 participants

@GSVarsha @pvital