From a31a6de1b1dc8a177d50084154fd3c5f75733082 Mon Sep 17 00:00:00 2001 From: Daniel Katz Date: Thu, 25 Dec 2025 00:59:52 +0200 Subject: [PATCH] Fix otel_span_attr merging across config levels (#107). Previously, when otel_span_attr directives were declared at both http and server levels, only the child level attributes were reported. This fix merges attributes from parent levels with child levels, ensuring all configured attributes are included in spans. --- src/http_module.cpp | 27 +++++++++++++++++ tests/test_otel.py | 70 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/src/http_module.cpp b/src/http_module.cpp index 78a5e896..ec38f138 100644 --- a/src/http_module.cpp +++ b/src/http_module.cpp @@ -959,6 +959,33 @@ char* mergeLocationConf(ngx_conf_t* cf, void* parent, void* child) if (conf->spanAttrs.elts == NULL) { conf->spanAttrs = prev->spanAttrs; + } else if (prev->spanAttrs.elts != NULL) { + ngx_array_t merged; + if (ngx_array_init(&merged, cf->pool, + prev->spanAttrs.nelts + conf->spanAttrs.nelts, + sizeof(SpanAttr)) != NGX_OK) { + return (char*)NGX_CONF_ERROR; + } + + auto prevAttrs = (SpanAttr*)prev->spanAttrs.elts; + for (ngx_uint_t i = 0; i < prev->spanAttrs.nelts; i++) { + auto attr = (SpanAttr*)ngx_array_push(&merged); + if (attr == NULL) { + return (char*)NGX_CONF_ERROR; + } + *attr = prevAttrs[i]; + } + + auto confAttrs = (SpanAttr*)conf->spanAttrs.elts; + for (ngx_uint_t i = 0; i < conf->spanAttrs.nelts; i++) { + auto attr = (SpanAttr*)ngx_array_push(&merged); + if (attr == NULL) { + return (char*)NGX_CONF_ERROR; + } + *attr = confAttrs[i]; + } + + conf->spanAttrs = merged; } auto mcf = getMainConf(cf); diff --git a/tests/test_otel.py b/tests/test_otel.py index fef771a5..2157b477 100644 --- a/tests/test_otel.py +++ b/tests/test_otel.py @@ -31,6 +31,7 @@ otel_trace on; {{ resource_attrs }} + {{ span_attrs or "" }} server { listen 127.0.0.1:18443 ssl; @@ -40,6 +41,7 @@ http2 on; server_name localhost; + {{ server_span_attrs or "" }} location /ok { return 200 "OK"; @@ -123,6 +125,13 @@ def get_attr(span, name): return getattr(value, value.WhichOneof("value")) +def get_all_attrs(span, name): + return [ + getattr(a.value, a.value.WhichOneof("value")) + for a in span.attributes if a.key == name + ] + + @pytest.fixture def client(nginx): urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) @@ -329,3 +338,64 @@ def test_tls_export(client, trace_service): assert client.get("http://127.0.0.1:18080/ok").status_code == 200 assert trace_service.get_span().name == "/ok" + + +@pytest.mark.parametrize( + "nginx_config", + [ + { + "span_attrs": """ + otel_span_attr config.level "http"; + otel_span_attr http.defined_at "http-block"; + """, + "server_span_attrs": """ + otel_span_attr config.level "server"; + otel_span_attr server.defined_at "server-block"; + """, + } + ], + indirect=True, +) +def test_span_attr_merge(client, trace_service): + assert client.get("http://127.0.0.1:18080/ok").status_code == 200 + + span = trace_service.get_span() + + http_level_attrs = get_all_attrs(span, "config.level") + assert "http" in http_level_attrs + assert "server" in http_level_attrs + + assert get_attr(span, "http.defined_at") == "http-block" + assert get_attr(span, "server.defined_at") == "server-block" + + +@pytest.mark.parametrize( + "nginx_config", + [ + { + "span_attrs": """ + otel_span_attr config.level "http"; + otel_span_attr http.defined_at "http-block"; + """, + "server_span_attrs": """ + otel_span_attr config.level "server"; + otel_span_attr server.defined_at "server-block"; + """, + } + ], + indirect=True, +) +def test_span_attr_merge_with_location(client, trace_service): + assert client.get("http://127.0.0.1:18080/custom").status_code == 200 + + span = trace_service.get_span() + + http_level_attrs = get_all_attrs(span, "config.level") + assert "http" in http_level_attrs + assert "server" in http_level_attrs + + assert get_attr(span, "http.defined_at") == "http-block" + assert get_attr(span, "server.defined_at") == "server-block" + + assert get_attr(span, "http.request.completion") == "OK" + assert get_attr(span, "http.request") == "GET /custom HTTP/1.1"