Update wsgi __init__.py to include HTTP_TARGET.

[philipcwhite]

Add HTTP_TARGET to server duration and active requests

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # Currently metrics exclude the HTTP_TARGET. This does not allow use to get specific resource counts or durations for visited endpoints. By adding HTTP_TARGET, we can now get detailed stats for individual page access. This will create additional metrics and increase cardinality. I feel like the granularity is needed to make the metric useful. It also brings this metric in-line with the spanmetrics connector.

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• [ x ] New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

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

• [ x ] Test A

Changed package and ran locally using auto-instrumentation->local collector with Prometheus exporter. Adding variables allowed prometheus to create correct metrics with targets.

Does This PR Require a Core Repo Change?

• Yes. - Link to PR:
• No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added
• Documentation has been updated

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: philipcwhite / name: Phil White (325b397, b7f9ecb, 3262449)

[aabmass]

@philipcwhite this is a purposeful omission since like you mentioned the cardinality can be unbounded. See the semantic conventions for http metrics https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/metrics/semantic_conventions/http-metrics.md.

Are you using this instrumentation directly or a more specific framework instrumentation that wraps it e.g. opentelemetry-instrumentation-flask. That framework should set http.route instead since that will be low cardinality. However I don't think this has been implemented in many (any?) of them.

[philipcwhite]

I am using this with Flask. Like I mentioned, this feature is present in the spanmetrics connector so it should be here as well. In spanmetrics it uses span_name.

{env="development", exported_job="pydemo", instance="otel-gateway.monitoring:8080", job="opentelmetry", service_name="pydemo", span_kind="SPAN_KIND_SERVER", span_name="/login", status_code="STATUS_CODE_UNSET", telemetry_auto_version="0.43b0", telemetry_sdk_language="python", telemetry_sdk_name="opentelemetry", telemetry_sdk_version="1.22.0"}

I assume this is a no-go as HTTP_TARGET is "deprecated" however it is still being used in spans...

HTTP_ROUTE may replace this according to the documentation however it does not seem work at this point in gathering endpoints like /login.html, /home, etc.

[philipcwhite]

I'm not sure if it matters but the target can be renamed to fit with the spec.

    if target is not None:
        #result[SpanAttributes.HTTP_TARGET] = target
        result[SpanAttributes.HTTP_ROUTE] = target

[sakthiraam]

Hi @aabmass , Good Day!

We are waiting for this http.route label in the metrics exposed by Python for some time now. We are able to get it in our Java (Auto Instrumentation) and Go Lang (Manual Instrumentation). Would be good if we can have the same level of support in Python as this information is crucial for troubleshooting and SLI/SLO based calculations.

And Yes, http.target might cause high cardinality as it includes query params. http.route should be correct label to use differentiate the request.

We have few issues in the repo pertaining to this request - #1457 , #1718 , #1963

Thank You @philipcwhite for the changes.

[philipcwhite]

it appears that using path info strips the variable.

result[SpanAttributes.HTTP_ROUTE] = environ.get("PATH_INFO")

for example if I hit the url http://172.16.6.136:5000/custom?hello=world
http_target shows /custom?hello=world
http_route shows /custom

This should be correct. The only problem I see if if someone does something silly like encoding a guid as a path.

[philipcwhite]

I'm a bit lost on this one after chatting with @aabmass on Slack. He suggested using the route data from Flask to populate HTTP_ROUTE. Its a nice idea but I ran into some issues that I don't know how to resolve. It looks like WSGI creates the metric on the first hit before flask can add the route to the wsgi environ variables. it picks up the correct variable on the return trip

wsgi None
before request /login
wsgi /login
wrapped span /login
172.16.6.136 - - [04/Jan/2024 12:48:38] "POST /login HTTP/1.1" 302 -

[philipcwhite]

I'm closing this PR as I can't figure out how to inject the flask routes into metrics. Hopefully someone else will take this up. Using wsgi only doesn't work either as it has no filter for excluded URLs. If the URLs could be filtered it may be a useable workaround. I could not get it to respect the OTEL_PYTHON_EXCLUDED_URLS env variable.

[philipcwhite]

closing