HTTP semantic convention stability migration for httplib #2648
Conversation
| active_requests_count_attrs = { | ||
| key: req_attrs[key] | ||
| for key in _active_requests_count_attrs.intersection(req_attrs.keys()) | ||
| def _parse_active_request_count_attrs( |
There was a problem hiding this comment.
opentelemetry-util-http is a utility package used only for http helper functions. We do not want to mix semantic convention logic in here. I suggest creating a method in the instrumentation file itself and calling the helper method within opentelemetry-instrumentation like we did in asgi.
| @@ -104,8 +110,15 @@ def trysetip(conn: http.client.HTTPConnection, loglevel=logging.DEBUG) -> bool: | |||
| stack_info=True, | |||
| ) | |||
| else: | |||
| _OpenTelemetrySemanticConventionStability._initialize() | |||
There was a problem hiding this comment.
We should call the initialize logic in the instrument() function.
| for span in spanlist: | ||
| span.set_attribute(SpanAttributes.NET_PEER_IP, ip) | ||
| if _report_old(stability_mode): |
There was a problem hiding this comment.
You can use the helper function here instead.
There was a problem hiding this comment.
I have tried to use this one, but in _set_http_peer_ip, it use this function
def set_string_attribute(result, key, value):
if value:
result[key] = value
but span do not support set attributes like this, so I write it like this
|
Hi are you still working on this? |
|
ah yeah, will keep working on this this week🫡. Sry, just a bit busy on work stuffs recently
Sent from Outlook for iOS
From: ***@***.*** ***@***.***> on behalf of Leighton Chen ***@***.***>Sent: Monday, July 22, 2024 5:50 PMTo: open-telemetry/opentelemetry-python-contrib ***@***.***>Cc: Zhihan Li ***@***.***>; Mention ***@***.***>Subject: Re: [open-telemetry/opentelemetry-python-contrib] HTTP semantic convention stability migration for httplib (PR #2648)
@zhihali
Hi are you still working on this?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
I don't believe |
Description
Part of this #2453
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
tox -e py312-test-util-httpDoes This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.