Include http.target in flask instrumentation metrics attributes

[ElementalWarrior]

As part of the semantic conventions, http.target should be included in the metrics attributes. However it is only present for span attributes.

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#parameterized-attributes

Currently it isn't part of the duration_attrs. Which it probably shouldn't until there is a solution to set the value to the flask url_rule instead of the raw input request:

• caller: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py#L295
• callee: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py#L236

[martinrw]

+1 We have the same requirement but Falcon rather than Flask

[sakthiraam]

+1 We have the same requirement for both Falcon and Flask.

Hi @ElementalWarrior , Good Day! I see you mentioned "Which it probably shouldn't until there is a solution to set the value to the flask url_rule instead of the raw input request".

Currently we are using the latest version of contrib opentelemetry-distro==0.41b0 in which we see the traces are already having the http.route and http.target labels

For Example, In traces it auto replaces like below

Static Resources

http.route -- /script/js/min.js to /script/path:filename

http.target -- /script/js/min.js to /script/js/min.js

Server Side resources

http.route -/about/xyz

http.target - /about/xyz.

I suppose the http.route will auto truncate any query params and keep only the route. With that is it ok to add http.route to the duration attributes ?

[alexmojaki]

open-telemetry/opentelemetry-specification#2818 changed the metric attribute name from http.target to http.route. The FastAPI instrumentation uses http.target. What's the recommended path forward?

1. Use http.target everywhere
2. Use http.route everywhere, except if http.target is already being used
3. Use http.route everywhere, updating places where http.target is already being used
4. Include both attributes - they should be the same and not increase the number of data points emitted
5. Use http.target for the 'old' attributes and http.route for the 'new' attributes, i.e. if OTEL_SEMCONV_STABILITY_OPT_IN is set.

This code:

opentelemetry-python-contrib/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py

Lines 62 to 95 in b94c590

	_server_duration_attrs_old = [
	SpanAttributes.HTTP_METHOD,
	SpanAttributes.HTTP_HOST,
	SpanAttributes.HTTP_SCHEME,
	SpanAttributes.HTTP_STATUS_CODE,
	SpanAttributes.HTTP_FLAVOR,
	SpanAttributes.HTTP_SERVER_NAME,
	SpanAttributes.NET_HOST_NAME,
	SpanAttributes.NET_HOST_PORT,
	]
	_server_duration_attrs_new = [
	ERROR_TYPE,
	HTTP_REQUEST_METHOD,
	HTTP_RESPONSE_STATUS_CODE,
	HTTP_ROUTE,
	NETWORK_PROTOCOL_VERSION,
	URL_SCHEME,
	]
	_server_active_requests_count_attrs_old = [
	SpanAttributes.HTTP_METHOD,
	SpanAttributes.HTTP_HOST,
	SpanAttributes.HTTP_SCHEME,
	SpanAttributes.HTTP_FLAVOR,
	SpanAttributes.HTTP_SERVER_NAME,
	SpanAttributes.NET_HOST_NAME,
	SpanAttributes.NET_HOST_PORT,
	]
	_server_active_requests_count_attrs_new = [
	HTTP_REQUEST_METHOD,
	URL_SCHEME,
	]

has HTTP_ROUTE in _server_duration_attrs_new, but it doesn't have http.target anywhere at all.

Also is there a reason to only include this in duration attributes, not the active requests count? I see that in flask at least the counter is incremented before the route is known.

[alexmojaki]

This is being worked on for Flask in #2621.

Should there be other issues for other server frameworks? I'd like to fix this in as many as possible.

[alexmojaki]

Additional wrinkle to the old/new attributes question - some instrumentations such as django don't have this concept at all and simply use this:

opentelemetry-python-contrib/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py

Lines 40 to 58 in b94c590

	# List of recommended metrics attributes
	_duration_attrs = {
	SpanAttributes.HTTP_METHOD,
	SpanAttributes.HTTP_HOST,
	SpanAttributes.HTTP_SCHEME,
	SpanAttributes.HTTP_STATUS_CODE,
	SpanAttributes.HTTP_FLAVOR,
	SpanAttributes.HTTP_SERVER_NAME,
	SpanAttributes.NET_HOST_NAME,
	SpanAttributes.NET_HOST_PORT,
	}
	_active_requests_count_attrs = {
	SpanAttributes.HTTP_METHOD,
	SpanAttributes.HTTP_HOST,
	SpanAttributes.HTTP_SCHEME,
	SpanAttributes.HTTP_FLAVOR,
	SpanAttributes.HTTP_SERVER_NAME,
	}

Should http.target and/or http.route be added to _duration_attrs?

[emdneto]

open-telemetry/opentelemetry-specification#2818 changed the metric attribute name from http.target to http.route. The FastAPI instrumentation uses http.target. What's the recommended path forward?

1. Use http.target everywhere
2. Use http.route everywhere, except if http.target is already being used
3. Use http.route everywhere, updating places where http.target is already being used
4. Include both attributes - they should be the same and not increase the number of data points emitted
5. Use http.target for the 'old' attributes and http.route for the 'new' attributes, i.e. if OTEL_SEMCONV_STABILITY_OPT_IN is set.

This code:

opentelemetry-python-contrib/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py

Lines 62 to 95 in b94c590

	_server_duration_attrs_old = [
	SpanAttributes.HTTP_METHOD,
	SpanAttributes.HTTP_HOST,
	SpanAttributes.HTTP_SCHEME,
	SpanAttributes.HTTP_STATUS_CODE,
	SpanAttributes.HTTP_FLAVOR,
	SpanAttributes.HTTP_SERVER_NAME,
	SpanAttributes.NET_HOST_NAME,
	SpanAttributes.NET_HOST_PORT,
	]
	_server_duration_attrs_new = [
	ERROR_TYPE,
	HTTP_REQUEST_METHOD,
	HTTP_RESPONSE_STATUS_CODE,
	HTTP_ROUTE,
	NETWORK_PROTOCOL_VERSION,
	URL_SCHEME,
	]
	_server_active_requests_count_attrs_old = [
	SpanAttributes.HTTP_METHOD,
	SpanAttributes.HTTP_HOST,
	SpanAttributes.HTTP_SCHEME,
	SpanAttributes.HTTP_FLAVOR,
	SpanAttributes.HTTP_SERVER_NAME,
	SpanAttributes.NET_HOST_NAME,
	SpanAttributes.NET_HOST_PORT,
	]
	_server_active_requests_count_attrs_new = [
	HTTP_REQUEST_METHOD,
	URL_SCHEME,
	]

has HTTP_ROUTE in _server_duration_attrs_new, but it doesn't have http.target anywhere at all.

Also is there a reason to only include this in duration attributes, not the active requests count? I see that in flask at least the counter is incremented before the route is known.

I think the transition is the best option as stated here. See #2453

[lzchen]

@alexmojaki

Yup, as @emdneto has stated, we will be following the guidance of the migration plan for only popular instrumentations. For example for fastapi which is built on top of asgi, we use the env var OTEL_SEMCONV_STABILITY_OPT_IN to conditionally set http.target or url.path/url.query for SPAN attributes. These are in turn filtered through expected attribute lists for metric attributes. You can find the old sem conv attributes here and new ones here for metrics that we follow.

[emdneto]

Fixed by #2621