From c80826cafad37696794caa2f1a69e40f1fb6c869 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Thu, 5 Dec 2024 11:12:41 +0200 Subject: [PATCH] Add B3 multi header propagation support (#495) * Add support for B3Multi headers propagation Some services expect [B3 Multi][1] headers as input information. To support that we need to be able to Inject them into upstream requests. Lowercase headers used to be [compatible][2] with Istio Envoy. Tests will be added as a separate commit later on. Solving https://github.com/open-telemetry/opentelemetry-cpp-contrib/issues/36 [1]: https://github.com/openzipkin/b3-propagation#multiple-headers [2]: https://github.com/open-telemetry/opentelemetry-go/issues/765 * add tests for b3 multi propagation --------- Co-authored-by: Vladimir Kuznichenkov Co-authored-by: Vladimir Kuznichenkov <5330267+kuzaxak@users.noreply.github.com> --- instrumentation/nginx/README.md | 3 +- instrumentation/nginx/src/otel_ngx_module.cpp | 66 +++++++++++++ instrumentation/nginx/src/propagate.cpp | 16 ++-- instrumentation/nginx/src/trace_context.h | 1 + .../nginx/test/backend/php/b3multi.php | 24 +++++ .../test/backend/simple_express/index.js | 30 +++++- instrumentation/nginx/test/conf/nginx.conf | 22 +++++ .../test/instrumentation_test.exs | 94 +++++++++++++++++-- 8 files changed, 234 insertions(+), 22 deletions(-) create mode 100644 instrumentation/nginx/test/backend/php/b3multi.php diff --git a/instrumentation/nginx/README.md b/instrumentation/nginx/README.md index 31478f43a..8c2a7b30d 100644 --- a/instrumentation/nginx/README.md +++ b/instrumentation/nginx/README.md @@ -169,7 +169,7 @@ be started. The default propagator is W3C. The same inheritance rules as [`proxy_set_header`](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_set_header) apply, which means this directive is applied at the current configuration level if and only if there are no `proxy_set_header` directives defined on a lower level. - **required**: `false` -- **syntax**: `opentelemetry_propagate` or `opentelemetry_propagate b3` +- **syntax**: `opentelemetry_propagate` or `opentelemetry_propagate b3` or `opentelemetry_propagate b3multi` - **block**: `http`, `server`, `location` ### `opentelemetry_capture_headers` @@ -255,6 +255,7 @@ The following nginx variables are set by the instrumentation: - `opentelemetry_context_traceparent` - [W3C trace context](https://www.w3.org/TR/trace-context/#trace-context-http-headers-format), e.g.: `00-0af7651916cd43dd8448eb211c80319c-b9c7c989f97918e1-01` - `opentelemetry_context_b3` - Trace context in the [B3 format](https://github.com/openzipkin/b3-propagation#single-header). Only set when using `opentelemetry_propagate b3`. +- `opentelemetry_sampled` - does current Span records information, "1" or "0" - `opentelemetry_trace_id` - Trace Id of the current span - `opentelemetry_span_id` - Span Id of the current span diff --git a/instrumentation/nginx/src/otel_ngx_module.cpp b/instrumentation/nginx/src/otel_ngx_module.cpp index 7023c580a..0aacebe52 100644 --- a/instrumentation/nginx/src/otel_ngx_module.cpp +++ b/instrumentation/nginx/src/otel_ngx_module.cpp @@ -143,6 +143,9 @@ OtelGetTraceId(ngx_http_request_t* req, ngx_http_variable_value_t* v, uintptr_t static ngx_int_t OtelGetSpanId(ngx_http_request_t* req, ngx_http_variable_value_t* v, uintptr_t data); +static ngx_int_t +OtelGetSampled(ngx_http_request_t* req, ngx_http_variable_value_t* v, uintptr_t data); + static ngx_http_variable_t otel_ngx_variables[] = { { ngx_string("otel_ctx"), @@ -176,6 +179,14 @@ static ngx_http_variable_t otel_ngx_variables[] = { NGX_HTTP_VAR_NOCACHEABLE | NGX_HTTP_VAR_NOHASH, 0, }, + { + ngx_string("opentelemetry_sampled"), + nullptr, + OtelGetSampled, + 0, + NGX_HTTP_VAR_NOCACHEABLE | NGX_HTTP_VAR_NOHASH, + 0, + }, ngx_http_null_variable, }; @@ -224,6 +235,46 @@ nostd::string_view WithoutOtelVarPrefix(ngx_str_t value) { return {(const char*)value.data + prefixLength, value.len - prefixLength}; } +static ngx_int_t +OtelGetSampled(ngx_http_request_t* req, ngx_http_variable_value_t* v, uintptr_t data) { + (void)data; + + if (!IsOtelEnabled(req)) { + v->valid = 0; + v->not_found = 1; + return NGX_OK; + } + + TraceContext* traceContext = GetTraceContext(req); + + if (traceContext == nullptr || !traceContext->request_span) { + ngx_log_error( + NGX_LOG_ERR, req->connection->log, 0, + "Unable to get trace context when getting span id"); + return NGX_OK; + } + + trace::SpanContext spanContext = traceContext->request_span->GetContext(); + + if (spanContext.IsValid()) { + u_char* isSampled = spanContext.trace_flags().IsSampled() ? (u_char*) "1" : (u_char*) "0"; + + v->len = 1; + v->valid = 1; + v->no_cacheable = 1; + v->not_found = 0; + v->data = isSampled; + } else { + v->len = 0; + v->valid = 0; + v->no_cacheable = 1; + v->not_found = 1; + v->data = nullptr; + } + + return NGX_OK; +} + static ngx_int_t OtelGetTraceContextVar(ngx_http_request_t* req, ngx_http_variable_value_t* v, uintptr_t data) { if (!IsOtelEnabled(req)) { @@ -778,6 +829,17 @@ std::vector B3PropagationVars() { }; } +std::vector B3MultiPropagationVars() { + return { + {"proxy_set_header", "x-b3-traceid", "$opentelemetry_trace_id"}, + {"proxy_set_header", "x-b3-spanid", "$opentelemetry_span_id"}, + {"proxy_set_header", "x-b3-sampled", "$opentelemetry_sampled"}, + {"fastcgi_param", "HTTP_B3_TRACEID", "$opentelemetry_trace_id"}, + {"fastcgi_param", "HTTP_B3_SPANID", "$opentelemetry_span_id"}, + {"fastcgi_param", "HTTP_B3_SAMPLED", "$opentelemetry_sampled"}, + }; +} + std::vector OtelPropagationVars() { return { {"proxy_set_header", "traceparent", "$opentelemetry_context_traceparent"}, @@ -798,6 +860,8 @@ char* OtelNgxSetPropagation(ngx_conf_t* conf, ngx_command_t*, void* locConf) { if (propagationType == "b3") { locationConf->propagationType = TracePropagationB3; + } else if (propagationType == "b3multi") { + locationConf->propagationType = TracePropagationB3Multi; } else if (propagationType == "w3c") { locationConf->propagationType = TracePropagationW3C; } else { @@ -811,6 +875,8 @@ char* OtelNgxSetPropagation(ngx_conf_t* conf, ngx_command_t*, void* locConf) { std::vector propagationVars; if (locationConf->propagationType == TracePropagationB3) { propagationVars = B3PropagationVars(); + } else if (locationConf->propagationType == TracePropagationB3Multi) { + propagationVars = B3MultiPropagationVars(); } else { propagationVars = OtelPropagationVars(); } diff --git a/instrumentation/nginx/src/propagate.cpp b/instrumentation/nginx/src/propagate.cpp index 4154af935..caab311c8 100644 --- a/instrumentation/nginx/src/propagate.cpp +++ b/instrumentation/nginx/src/propagate.cpp @@ -42,11 +42,6 @@ static bool FindHeader(ngx_http_request_t* req, nostd::string_view key, nostd::s } -static bool HasHeader(ngx_http_request_t* req, nostd::string_view header) { - nostd::string_view value; - return FindHeader(req, header, &value); -} - class TextMapCarrierNgx : public opentelemetry::context::propagation::TextMapCarrier { public: @@ -79,12 +74,9 @@ opentelemetry::context::Context ExtractContext(OtelCarrier* carrier) { case TracePropagationW3C: { return OtelW3CPropagator().Extract(textMapCarrier, root); } + case TracePropagationB3Multi: case TracePropagationB3: { - if (HasHeader(carrier->req, "b3")) { - return OtelB3Propagator().Extract(textMapCarrier, root); - } - - return OtelB3MultiPropagator().Extract(textMapCarrier, root); + return OtelB3Propagator().Extract(textMapCarrier, root); } default: return root; @@ -104,6 +96,10 @@ void InjectContext(OtelCarrier* carrier, opentelemetry::context::Context context OtelB3Propagator().Inject(textMapCarrier, context); break; } + case TracePropagationB3Multi: { + OtelB3MultiPropagator().Inject(textMapCarrier, context); + break; + } default: break; } diff --git a/instrumentation/nginx/src/trace_context.h b/instrumentation/nginx/src/trace_context.h index 2d4aeca56..7411a6143 100644 --- a/instrumentation/nginx/src/trace_context.h +++ b/instrumentation/nginx/src/trace_context.h @@ -15,6 +15,7 @@ enum TracePropagationType { TracePropagationUnset, TracePropagationW3C, TracePropagationB3, + TracePropagationB3Multi, }; struct TraceContext { diff --git a/instrumentation/nginx/test/backend/php/b3multi.php b/instrumentation/nginx/test/backend/php/b3multi.php new file mode 100644 index 000000000..e0a922038 --- /dev/null +++ b/instrumentation/nginx/test/backend/php/b3multi.php @@ -0,0 +1,24 @@ + $b3_trace_id, + "x-b3-spanid" => $b3_span_id, + "x-b3-sampled" => $b3_sampled + ))); +?> diff --git a/instrumentation/nginx/test/backend/simple_express/index.js b/instrumentation/nginx/test/backend/simple_express/index.js index f9e6b366c..9463066cd 100644 --- a/instrumentation/nginx/test/backend/simple_express/index.js +++ b/instrumentation/nginx/test/backend/simple_express/index.js @@ -1,6 +1,6 @@ -const express = require('express') -const app = express() -const port = 3500 +const express = require('express'); +const app = express(); +const port = 3500; const traceparentRegex = /00-[0-9a-f]{32}-[0-9a-f]{16}-0[0-1]/; @@ -22,6 +22,30 @@ app.get("/b3", (req, res) => { res.json({"b3": header}); }); +app.get("/b3multi", (req, res) => { + let traceId = req.header("x-b3-traceid"); + let spanId = req.header("x-b3-spanid"); + let sampled = req.header("x-b3-sampled"); + + if (!/^([0-9a-f]{32}|[0-9a-f]{16})$/.test(traceId)) { + throw "Missing x-b3-traceid header"; + } + + if (!/^[0-9a-f]{16}$/.test(spanId)) { + throw "Missing x-b3-spanid header"; + } + + if (!["0", "1"].includes(sampled)) { + throw "Missing x-b3-sampled header"; + } + + res.json({ + "x-b3-traceid": traceId, + "x-b3-spanid": spanId, + "x-b3-sampled": sampled + }); +}); + app.get("/off", (req, res) => { if (req.header("traceparent") !== undefined) { throw "Found traceparent header, but expected none"; diff --git a/instrumentation/nginx/test/conf/nginx.conf b/instrumentation/nginx/test/conf/nginx.conf index f77f38005..4798d79fe 100644 --- a/instrumentation/nginx/test/conf/nginx.conf +++ b/instrumentation/nginx/test/conf/nginx.conf @@ -42,6 +42,12 @@ http { proxy_pass http://node-backend/b3; } + location = /b3multi { + opentelemetry_operation_name test_b3multi; + opentelemetry_propagate b3multi; + proxy_pass http://node-backend/b3multi; + } + location = /off { opentelemetry off; proxy_pass http://node-backend/off; @@ -126,6 +132,22 @@ http { return 200 ""; } + location = /b3multi.php { + include /etc/nginx/fastcgi_params; + root /var/www/html/php; + opentelemetry_operation_name php_fpm_b3multi; + opentelemetry_propagate b3multi; + fastcgi_pass php-backend:9000; + } + + location = /b3.php { + include /etc/nginx/fastcgi_params; + root /var/www/html/php; + opentelemetry_operation_name php_fpm_b3; + opentelemetry_propagate b3; + fastcgi_pass php-backend:9000; + } + location ~ \.php$ { include /etc/nginx/fastcgi_params; root /var/www/html/php; diff --git a/instrumentation/nginx/test/instrumentation/test/instrumentation_test.exs b/instrumentation/nginx/test/instrumentation/test/instrumentation_test.exs index 122b8b21d..f8569aac1 100644 --- a/instrumentation/nginx/test/instrumentation/test/instrumentation_test.exs +++ b/instrumentation/nginx/test/instrumentation/test/instrumentation_test.exs @@ -316,29 +316,79 @@ defmodule InstrumentationTest do test_parent_span("#{@host}/app.php", ctx) end - test "HTTP upstream | test b3 injection", %{trace_file: trace_file} do - %HTTPoison.Response{status_code: status} = HTTPoison.get!("#{@host}/b3") + test "HTTP upstream | b3 injection", %{trace_file: trace_file} do + %HTTPoison.Response{status_code: status, body: body} = HTTPoison.get!("#{@host}/b3") + + %{"b3" => b3} = Jason.decode!(body) + [trace_id, span_id, _] = String.split(b3, "-") [trace] = read_traces(trace_file, 1) [span] = collect_spans(trace) assert status == 200 assert span["parentSpanId"] == "" + assert span["traceId"] == trace_id + assert span["spanId"] == span_id assert span["name"] == "test_b3" end - test "PHP-FPM upstream | test b3 injection", %{trace_file: trace_file} do - %HTTPoison.Response{status_code: status} = HTTPoison.get!("#{@host}/b3") + test "PHP-FPM upstream | b3 propagation", %{trace_file: trace_file} do + input_trace_id = "aad85b4f655feed4d594a01cfa6a1d64" + input_span_id = "2a9d49c3e3b7c461" + + %HTTPoison.Response{status_code: status, body: body} = + HTTPoison.get!("#{@host}/b3.php", [ + {"b3", "#{input_trace_id}-#{input_span_id}-1"} + ]) + + %{"b3" => b3} = Jason.decode!(body) + [trace_id, span_id, _] = String.split(b3, "-") + + assert input_trace_id == trace_id + assert input_span_id != span_id [trace] = read_traces(trace_file, 1) [span] = collect_spans(trace) assert status == 200 - assert span["parentSpanId"] == "" - assert span["name"] == "test_b3" + assert span["parentSpanId"] == input_span_id + assert span["traceId"] == input_trace_id + assert span["spanId"] == span_id + assert span["name"] == "php_fpm_b3" + end + + test "PHP-FPM upstream | multiheader b3 propagation", %{trace_file: trace_file} do + input_trace_id = "aad85b4f655feed4d594a01cfa6a1d64" + input_span_id = "2a9d49c3e3b7c461" + + %HTTPoison.Response{status_code: status, body: body} = + HTTPoison.get!("#{@host}/b3multi.php", [ + {"X-B3-TraceId", input_trace_id}, + {"X-B3-SpanId", input_span_id}, + {"X-B3-Sampled", "1"} + ]) + + %{ + "x-b3-traceid" => trace_id, + "x-b3-spanid" => span_id, + "x-b3-sampled" => sampled + } = Jason.decode!(body) + + assert sampled == "1" + assert input_trace_id == trace_id + assert input_span_id != span_id + + [trace] = read_traces(trace_file, 1) + [span] = collect_spans(trace) + + assert status == 200 + assert span["parentSpanId"] == input_span_id + assert span["traceId"] == input_trace_id + assert span["spanId"] == span_id + assert span["name"] == "php_fpm_b3multi" end - test "HTTP upstream | test b3 propagation", %{trace_file: trace_file} do + test "HTTP upstream | b3 propagation", %{trace_file: trace_file} do parent_span_id = "2a9d49c3e3b7c461" input_trace_id = "aad85b4f655feed4d594a01cfa6a1d62" @@ -361,7 +411,7 @@ defmodule InstrumentationTest do assert span["name"] == "test_b3" end - test "HTTP upstream | multiheader b3 propagation", %{trace_file: trace_file} do + test "HTTP upstream | multiheader b3 extraction", %{trace_file: trace_file} do parent_span_id = "2a9d49c3e3b7c461" input_trace_id = "aad85b4f655feed4d594a01cfa6a1d62" @@ -386,6 +436,34 @@ defmodule InstrumentationTest do assert span["name"] == "test_b3" end + test "HTTP upstream | multiheader b3 injection", %{trace_file: trace_file} do + span_id = "2a9d49c3e3b7c462" + trace_id = "aad85b4f655feed4d594a01cfa6a1d63" + + %HTTPoison.Response{status_code: status, body: body} = + HTTPoison.get!("#{@host}/b3multi", [ + {"b3", "#{trace_id}-#{span_id}-1"} + ]) + + [trace] = read_traces(trace_file, 1) + [span] = collect_spans(trace) + + %{ + "x-b3-traceid" => header_trace_id, + "x-b3-spanid" => header_span_id, + "x-b3-sampled" => header_sampled + } = Jason.decode!(body) + + assert header_trace_id == trace_id + assert header_span_id != span_id + assert header_sampled == "1" + + assert status == 200 + assert span["parentSpanId"] == span_id + assert span["spanId"] != span_id + assert span["name"] == "test_b3multi" + end + test "Accessing a file produces a span", %{trace_file: trace_file} do %HTTPoison.Response{status_code: status, body: body} = HTTPoison.get!("#{@host}/files/content.txt")