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

HTTP semantic convention stability migration for urllib3 #2715

Merged
merged 11 commits into from
Jul 23, 2024

Conversation

emdneto
Copy link
Member

@emdneto emdneto commented Jul 16, 2024

Description

Part of #2453
Fixes #2681

Checklist

  • Add semconv status as migration for urllib3 in instrumentations README
  • Populate {method} as HTTP when nonstandard method (sanitize_method)
  • set request HTTP method attribute as _OTHER when nonstandard http method and for new semconv also set http.request.method_original with the original nonstandard method.
  • Set schema_url based on the opt-in mode
  • Create histograms based on the opt-in mode (different metric names so no dup attributes)
  • Set metrics attributes based on the opt-in mode
  • Set span attributes based on the opt-in mode

At the current implementation, some of the required/recommended semconv span attributes aren't being set:

  • server.address , server.port
  • network.protocol.verison

@github-actions github-actions bot requested review from ocelotl and shalevr July 16, 2024 22:36
@emdneto emdneto self-assigned this Jul 16, 2024
@emdneto emdneto marked this pull request as ready for review July 22, 2024 00:07
@emdneto emdneto requested a review from a team July 22, 2024 00:07
@emdneto emdneto requested a review from lzchen July 22, 2024 00:46
@lzchen
Copy link
Contributor

lzchen commented Jul 22, 2024

At the current implementation, some of the required/recommended semconv span attributes aren't being set:

server.address , server.port
network.protocol.verison

Can probably create a separate issue for these.

@lzchen lzchen merged commit 92da527 into open-telemetry:main Jul 23, 2024
383 checks passed
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.

opentelemetry-instrumentation-urllib3: HTTP semantic convention stability migration
4 participants