Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/24706 )
Change subject: IMPALA-14752: Support W3C trace context propagation for hs2-http ...................................................................... Patch Set 2: (8 comments) http://gerrit.cloudera.org:8080/#/c/24706/1/be/src/observe/otel-propagation.h File be/src/observe/otel-propagation.h: http://gerrit.cloudera.org:8080/#/c/24706/1/be/src/observe/otel-propagation.h@29 PS1, Line 29: public: > Done Done http://gerrit.cloudera.org:8080/#/c/24706/1/be/src/observe/otel-propagation.cc File be/src/observe/otel-propagation.cc: http://gerrit.cloudera.org:8080/#/c/24706/1/be/src/observe/otel-propagation.cc@53 PS1, Line 53: HttpHeaderCarrier carrier(traceparent, tracestate); : context::Context ctx; > Done Done http://gerrit.cloudera.org:8080/#/c/24706/1/be/src/observe/otel-propagation.cc@34 PS1, Line 34: if (key == trace::propagation::kTraceParent) { : return traceparent_; : } : if (key == trace::propagation::kTraceState) { : return tracestate_; : } : return ""; : } : : void HttpHeaderCarrier::Set(nostd::string_view key, : nostd::string_view value) noexcept {} : : trace::SpanContext ExtractSpanContextFromHttpHeaders(nostd::string_view traceparent, : nostd::string_view tracestate) { : if (traceparent.empty()) { : return trace::SpanContext::GetInvalid(); : } : : trace::propagation::HttpTraceContext propagator; : HttpHeaderCarrier carrier(traceparent, tracestate); : context::Context ctx; : > Done Done http://gerrit.cloudera.org:8080/#/c/24706/1/be/src/observe/otel-test.cc File be/src/observe/otel-test.cc: http://gerrit.cloudera.org:8080/#/c/24706/1/be/src/observe/otel-test.cc@371 PS1, Line 371: > Done Done http://gerrit.cloudera.org:8080/#/c/24706/1/be/src/observe/otel-test.cc@386 PS1, Line 386: : EXPECT_EQ(tracestate, ctx.trace_state()->ToHeader()); > Done Done http://gerrit.cloudera.org:8080/#/c/24706/2/be/src/observe/otel-trace-manager.cc File be/src/observe/otel-trace-manager.cc: http://gerrit.cloudera.org:8080/#/c/24706/2/be/src/observe/otel-trace-manager.cc@186 PS2, Line 186: std::string_view http_tracestate = client_request_state_->http_tracestate(); Move this declaration down one line into the "if" statement since this variable is only used inside that block. http://gerrit.cloudera.org:8080/#/c/24706/1/be/src/service/client-request-state.h File be/src/service/client-request-state.h: http://gerrit.cloudera.org:8080/#/c/24706/1/be/src/service/client-request-state.h@248 PS1, Line 248: std::shared_ptr<ImpalaServer::SessionState> session() const { return session_; } : : /// Returns the W3C Trace Context traceparent header from the hs2-http request that : /// submitted this query, if any. : std::string_view http_traceparent() const { return http_traceparent_; } : : /// Returns the W3C Trace Context tracestate header from the hs2-http r > Done Done http://gerrit.cloudera.org:8080/#/c/24706/1/be/src/service/client-request-state.h@640 PS1, Line 640: std::shared_ptr<ImpalaServer::SessionState> session_; : : /// W3C Trace Context headers > I think the ConnectionContext outlives a single HTTP request (it is per-con That sound accurate, comment withdrawn. -- To view, visit http://gerrit.cloudera.org:8080/24706 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieac548f4f5f613ba116d1a79d95e31b7a1a37b97 Gerrit-Change-Number: 24706 Gerrit-PatchSet: 2 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Comment-Date: Thu, 27 Aug 2026 20:01:40 +0000 Gerrit-HasComments: Yes
