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

Remove HttpStatusConverter #4447

Closed
breedx-splk opened this issue Oct 20, 2021 · 1 comment
Closed

Remove HttpStatusConverter #4447

breedx-splk opened this issue Oct 20, 2021 · 1 comment
Assignees
@breedx-splk
Labels
enhancement New feature or request

Comments

@breedx-splk
Copy link
Contributor

In #4403, it was brought up that the HttpStatusConverter is a more legacy interface from the tracing api. The tracing api has not fully been removed, but we are getting close. Once that happens, we think we want to remove HttpStatusConverter.

Even with the tracers removed, this will be a bit tricky due to a few items:

  • Some tests rely on it
  • HttpUrlConnectionInstrumentation still uses it
  • Don't report 400 level as error for SERVER spans #4403 introduces a dependency on it from HttpSpanStatusExtractor. We think this can be mitigated by splitting HttpSpanStatusExtractor into HttpServerSpanStatusExtractor and HttpClientSpanStatusExtractor and inlining what's in HttpStatusConverter (perhaps exposing it thru a static method for use as a utility).
@breedx-splk breedx-splk added the enhancement New feature or request label Oct 20, 2021
@breedx-splk breedx-splk self-assigned this Oct 20, 2021
@breedx-splk breedx-splk mentioned this issue Oct 20, 2021
Merged
@mateuszrzeszutek
Copy link
Member

Looks like this was done in #5268

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@breedx-splk breedx-splk

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mateuszrzeszutek @breedx-splk