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

Fix status_code Tag type and add further Tag definitions #783

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nefilim
Copy link
Contributor

@nefilim nefilim commented Apr 20, 2023

These are really following the OpenTelemetry Semantic Attribute specification, so not sure this is the best place for them but there are no tags defined under the OpenTelemetry module currently.

Also - not sure how important it is to preserve backward compatibility for def status_code - could add an overloaded function with String parameter if deemed necessary.

This PR will be followed with another PR in natchez-http4s to update invocation to status_code, once it's in the correct format (int64 vs string) - at least the AWS Xray collector will correctly export the value and it will be available in the trace data, currently it's just showing as 0.

@nefilim nefilim changed the title Add further Tag definitions Fix status_code Tag type and add further Tag definitions Apr 21, 2023
@mpilquist
Copy link
Member

Looks good. I'd avoid the change to status_code signature if possible and if not, then add the overload for biocompat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants