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

flask: add http.target and http.route to metric attributes #2621

Merged
merged 27 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
3e0ba7d
add metric attribute
GonzaloGuasch Jun 19, 2024
86fe2e6
Update changelog && change variable in test
GonzaloGuasch Jun 24, 2024
19fe714
Merge branch 'main' into feature/http-route-in-metric
GonzaloGuasch Jun 24, 2024
fc48830
Remove append from test
GonzaloGuasch Jun 24, 2024
b2c8849
Change to new HTTP_ROUTE semconv & linter fix
GonzaloGuasch Jun 24, 2024
447624a
Merge branch 'main' into feature/http-route-in-metric
GonzaloGuasch Jul 1, 2024
6633306
Update changelog
GonzaloGuasch Jul 1, 2024
566023e
Merge branch 'main' into feature/http-route-in-metric
GonzaloGuasch Jul 1, 2024
36f2078
Update changelog
GonzaloGuasch Jul 1, 2024
c2db445
Merge branch 'main' into feature/http-route-in-metric
GonzaloGuasch Jul 2, 2024
6a30b1f
Merge branch 'main' into feature/http-route-in-metric
GonzaloGuasch Jul 4, 2024
b83d7ed
Merge branch 'main' into feature/http-route-in-metric
GonzaloGuasch Jul 8, 2024
edd0ad7
Merge branch 'main' into feature/http-route-in-metric
GonzaloGuasch Jul 11, 2024
335657e
Update CHANGELOG.md
GonzaloGuasch Jul 11, 2024
077b410
Merge branch 'main' into feature/http-route-in-metric
GonzaloGuasch Jul 13, 2024
304c88b
Add http.target in metric attribute
GonzaloGuasch Jul 17, 2024
c8d9555
Merge branch 'main' into feature/http-route-in-metric
GonzaloGuasch Jul 18, 2024
7d3c150
http.target in old sem metric attribute
GonzaloGuasch Jul 18, 2024
d1c747e
Change http.target to SpanAttributes.HTTP_TARGET
GonzaloGuasch Jul 18, 2024
547c99c
Merge branch 'main' into feature/http-route-in-metric
GonzaloGuasch Jul 18, 2024
93daa35
Change http.route to SpanAttributes.HTTP_ROUTE
GonzaloGuasch Jul 18, 2024
d4a3141
Merge branch 'main' into feature/http-route-in-metric
emdneto Jul 19, 2024
588199d
Change HTTP_ROUTE import module
GonzaloGuasch Jul 19, 2024
cea918d
Change env variable to nonlocal var
GonzaloGuasch Jul 19, 2024
122ca57
change flask variable str method
GonzaloGuasch Jul 19, 2024
c4cc8b3
remove if in flask.request.url_rule
GonzaloGuasch Jul 20, 2024
ec4ff02
Merge branch 'main' into feature/http-route-in-metric
lzchen Jul 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- `opentelemetry-instrumentation-flask` Add `http.route` to metric attributes
xrmx marked this conversation as resolved.
Show resolved Hide resolved
([#2506](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2506))
- `opentelemetry-sdk-extension-aws` Register AWS resource detectors under the
`opentelemetry_resource_detector` entry point
([#2382](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2382))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,11 @@ def response_hook(span: Span, status: str, response_headers: List):
)
from opentelemetry.instrumentation.utils import _start_internal_or_server_span
from opentelemetry.metrics import get_meter
from opentelemetry.semconv.attributes.http_attributes import HTTP_ROUTE
from opentelemetry.semconv.metrics import MetricInstruments
from opentelemetry.semconv.metrics.http_metrics import (
HTTP_SERVER_REQUEST_DURATION,
)
from opentelemetry.semconv.trace import SpanAttributes
from opentelemetry.util.http import (
get_excluded_urls,
parse_excluded_urls,
Expand All @@ -284,6 +284,7 @@ def response_hook(span: Span, status: str, response_headers: List):
_ENVIRON_ACTIVATION_KEY = "opentelemetry-flask.activation_key"
_ENVIRON_REQCTX_REF_KEY = "opentelemetry-flask.reqctx_ref_key"
_ENVIRON_TOKEN = "opentelemetry-flask.token"
_ENVIRON_REQUEST_ROUTE_KEY = "opentelemetry-flask.request-route_key"

_excluded_urls_from_env = get_excluded_urls("FLASK")

Expand Down Expand Up @@ -346,6 +347,11 @@ def _start_response(status, response_headers, *args, **kwargs):
excluded_urls is None
or not excluded_urls.url_disabled(flask.request.url)
):
if flask.request.url_rule:
wrapped_app_environ[_ENVIRON_REQUEST_ROUTE_KEY] = str(
GonzaloGuasch marked this conversation as resolved.
Show resolved Hide resolved
flask.request.url_rule
)

span = flask.request.environ.get(_ENVIRON_SPAN_KEY)

propagator = get_global_response_propagator()
Expand Down Expand Up @@ -388,13 +394,25 @@ def _start_response(status, response_headers, *args, **kwargs):
duration_attrs_old = otel_wsgi._parse_duration_attrs(
attributes, _HTTPStabilityMode.DEFAULT
)

if wrapped_app_environ.get(_ENVIRON_REQUEST_ROUTE_KEY, None):
emdneto marked this conversation as resolved.
Show resolved Hide resolved
duration_attrs_old[HTTP_ROUTE] = wrapped_app_environ.get(
GonzaloGuasch marked this conversation as resolved.
Show resolved Hide resolved
_ENVIRON_REQUEST_ROUTE_KEY
)

duration_histogram_old.record(
max(round(duration_s * 1000), 0), duration_attrs_old
)
if duration_histogram_new:
duration_attrs_new = otel_wsgi._parse_duration_attrs(
attributes, _HTTPStabilityMode.HTTP
)

if wrapped_app_environ.get(_ENVIRON_REQUEST_ROUTE_KEY, None):
duration_attrs_new[HTTP_ROUTE] = wrapped_app_environ.get(
_ENVIRON_REQUEST_ROUTE_KEY
)

duration_histogram_new.record(
max(duration_s, 0), duration_attrs_new
)
Expand Down Expand Up @@ -425,7 +443,7 @@ def _before_request():
if flask.request.url_rule:
# For 404 that result from no route found, etc, we
# don't have a url_rule.
attributes[SpanAttributes.HTTP_ROUTE] = flask.request.url_rule.rule
attributes[HTTP_ROUTE] = flask.request.url_rule.rule
GonzaloGuasch marked this conversation as resolved.
Show resolved Hide resolved
span, token = _start_internal_or_server_span(
tracer=tracer,
span_name=span_name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ def expected_attributes_new(override_attributes):
return default_attributes


_server_duration_attrs_old_copy = _server_duration_attrs_old.copy()
_server_duration_attrs_old_copy.append("http.route")

_expected_metric_names_old = [
"http.server.active_requests",
"http.server.duration",
Expand All @@ -95,7 +98,7 @@ def expected_attributes_new(override_attributes):
]
_recommended_metrics_attrs_old = {
"http.server.active_requests": _server_active_requests_count_attrs_old,
"http.server.duration": _server_duration_attrs_old,
"http.server.duration": _server_duration_attrs_old_copy,
}
_recommended_metrics_attrs_new = {
"http.server.active_requests": _server_active_requests_count_attrs_new,
Expand All @@ -109,7 +112,7 @@ def expected_attributes_new(override_attributes):
)
_recommended_metrics_attrs_both = {
"http.server.active_requests": _server_active_requests_count_attrs_both,
"http.server.duration": _server_duration_attrs_old,
"http.server.duration": _server_duration_attrs_old_copy,
"http.server.request.duration": _server_duration_attrs_new,
}

Expand Down Expand Up @@ -570,6 +573,7 @@ def test_basic_metric_success(self):
self.client.get("/hello/756")
expected_duration_attributes = {
"http.method": "GET",
"http.route": "/hello/<int:helloid>",
GonzaloGuasch marked this conversation as resolved.
Show resolved Hide resolved
"http.host": "localhost",
"http.scheme": "http",
"http.flavor": "1.1",
Expand Down Expand Up @@ -597,6 +601,7 @@ def test_basic_metric_success_new_semconv(self):
expected_duration_attributes = {
"http.request.method": "GET",
"url.scheme": "http",
"http.route": "/hello/<int:helloid>",
"network.protocol.version": "1.1",
"http.response.status_code": 200,
}
Expand Down