diff --git a/system/jlib/jtrace.cpp b/system/jlib/jtrace.cpp index 714ec9e5f85..4c62feb73ad 100644 --- a/system/jlib/jtrace.cpp +++ b/system/jlib/jtrace.cpp @@ -594,66 +594,69 @@ class CSpan : public CInterfaceOf<ISpan> } /** - * Retrieves the Span's context as key/value pairs into the provided IProperties. - * Optionally, output follows OpenTelemetry Span context format for propogation + * Retrieves the Span's client headers traceparent and tracestate + * Output follows OpenTelemetry Span context format for propogation * accross process boundaries. * - * @param ctxProps IProperties container for span context key/value pairs. - * @param otelFormatted If true, output follows OpenTelemetry Span context format. - * @return True if the span context was successfully retrieved, false otherwise. + * @param clientHeaders IProperties container for client headers. */ - bool getSpanContext(IProperties * ctxProps, bool otelFormatted) const override + void getClientHeaders(IProperties * clientHeaders) const { - if (ctxProps == nullptr) - return false; + if (clientHeaders == nullptr) + return; - ctxProps->setNonEmptyProp(kGlobalIdHttpHeaderName, queryGlobalId()); + clientHeaders->setNonEmptyProp(kGlobalIdHttpHeaderName, queryGlobalId()); - if (otelFormatted) - { - //The localid is passed as the callerid for the client request.... - ctxProps->setNonEmptyProp(kCallerIdHttpHeaderName, queryLocalId()); - } - else - { - ctxProps->setNonEmptyProp(kCallerIdHttpHeaderName, queryCallerId()); - } + //The localid is passed as the callerid for the client request.... + clientHeaders->setNonEmptyProp(kCallerIdHttpHeaderName, queryLocalId()); if (span == nullptr) - return false; + return; - if (otelFormatted) - { - if (isEmptyString(traceID.get()) || isEmptyString(spanID.get()) || isEmptyString(traceFlags.get())) - return false; + if (isEmptyString(traceID.get()) || isEmptyString(spanID.get()) || isEmptyString(traceFlags.get())) + return; - //The traceparent header uses the version-trace_id-parent_id-trace_flags format where: - //version is always 00. trace_id is a hex-encoded trace id. span_id is a hex-encoded span id. trace_flags is a hex-encoded 8-bit field that contains tracing flags such as sampling, trace level, etc. - //Example: "traceparent", "00-beca49ca8f3138a2842e5cf21402bfff-4b960b3e4647da3f-01" + //The traceparent header uses the version-trace_id-parent_id-trace_flags format where: + //version is always 00. trace_id is a hex-encoded trace id. span_id is a hex-encoded span id. trace_flags is a hex-encoded 8-bit field that contains tracing flags such as sampling, trace level, etc. + //Example: "traceparent", "00-beca49ca8f3138a2842e5cf21402bfff-4b960b3e4647da3f-01" - StringBuffer contextHTTPHeader; - //https://www.w3.org/TR/trace-context/#header-name - contextHTTPHeader.append("00-").append(traceID.get()).append("-").append(spanID.get()).append("-").append(traceFlags.get()); - ctxProps->setProp(opentelemetry::trace::propagation::kTraceParent.data(), contextHTTPHeader.str()); + StringBuffer contextHTTPHeader; + //https://www.w3.org/TR/trace-context/#header-name + contextHTTPHeader.append("00-").append(traceID.get()).append("-").append(spanID.get()).append("-").append(traceFlags.get()); + clientHeaders->setProp(opentelemetry::trace::propagation::kTraceParent.data(), contextHTTPHeader.str()); - //The main purpose of the tracestate HTTP header is to provide additional vendor-specific trace identification - // information across different distributed tracing systems and is a companion header for the traceparent field. - // It also conveys information about the request’s position in multiple distributed tracing graphs. + //The main purpose of the tracestate HTTP header is to provide additional vendor-specific trace identification + // information across different distributed tracing systems and is a companion header for the traceparent field. + // It also conveys information about the request’s position in multiple distributed tracing graphs. - //https://www.w3.org/TR/trace-context/#trace-context-http-headers-format - //StringBuffer traceStateHTTPHeader; - //traceStateHTTPHeader.append("hpcc=").append(spanID.get()); + //https://www.w3.org/TR/trace-context/#trace-context-http-headers-format + //StringBuffer traceStateHTTPHeader; + //traceStateHTTPHeader.append("hpcc=").append(spanID.get()); - ctxProps->setNonEmptyProp(opentelemetry::trace::propagation::kTraceState.data(), span->GetContext().trace_state()->ToHeader().c_str()); - } - else - { - ctxProps->setNonEmptyProp("traceID", traceID.get()); - ctxProps->setNonEmptyProp("spanID", spanID.get()); - ctxProps->setNonEmptyProp("traceFlags", traceFlags.get()); - } + clientHeaders->setNonEmptyProp(opentelemetry::trace::propagation::kTraceState.data(), span->GetContext().trace_state()->ToHeader().c_str()); + } - return true; + /** + * Retrieves the Span's context as key/value pairs into the provided IProperties. + * Optionally, output follows OpenTelemetry Span context format for propogation + * accross process boundaries. + * + * @param ctxProps IProperties container for span context key/value pairs. + */ + void getSpanContext(IProperties * ctxProps) const override + { + if (ctxProps == nullptr) + return; + + ctxProps->setNonEmptyProp(kGlobalIdHttpHeaderName, queryGlobalId()); + ctxProps->setNonEmptyProp(kCallerIdHttpHeaderName, queryCallerId()); + + if (span == nullptr) + return; + + ctxProps->setNonEmptyProp("traceID", traceID.get()); + ctxProps->setNonEmptyProp("spanID", spanID.get()); + ctxProps->setNonEmptyProp("traceFlags", traceFlags.get()); } opentelemetry::v1::trace::SpanContext querySpanContext() const @@ -783,7 +786,8 @@ class CNullSpan : public CInterfaceOf<ISpan> virtual void setSpanAttributes(const IProperties * attributes) override {} virtual void addSpanEvent(const char * eventName) override {} virtual void addSpanEvent(const char * eventName, IProperties * attributes) override {}; - virtual bool getSpanContext(IProperties * ctxProps, bool otelFormatted) const override { return false; } + virtual void getSpanContext(IProperties * ctxProps) const override {} + virtual void getClientHeaders(IProperties * clientHeaders) const override {} virtual void toString(StringBuffer & out) const override {} virtual void getLogPrefix(StringBuffer & out) const override {} @@ -818,19 +822,13 @@ class CChildSpan : public CSpan } public: - virtual bool getSpanContext(IProperties * ctxProps, bool otelFormatted) const override + virtual void getSpanContext(IProperties * ctxProps) const override { - //MORE: It is not clear what this return value represents, and whether it would ever be checked. - bool ok = CSpan::getSpanContext(ctxProps, otelFormatted); + CSpan::getSpanContext(ctxProps); - if (ok && !otelFormatted) - { - Owned<IProperties> localParentSpanCtxProps = createProperties(); - localParentSpan->getSpanContext(localParentSpanCtxProps, false); - ctxProps->setNonEmptyProp("localParentSpanID", localParentSpanCtxProps->queryProp("spanID")); - } - - return ok; + Owned<IProperties> localParentSpanCtxProps = createProperties(); + localParentSpan->getSpanContext(localParentSpanCtxProps); + ctxProps->setNonEmptyProp("localParentSpanID", localParentSpanCtxProps->queryProp("spanID")); } virtual const char* queryGlobalId() const override @@ -951,11 +949,11 @@ class CServerSpan : public CSpan } } - bool getSpanContext(IProperties * ctxProps, bool otelFormatted) const override + void getSpanContext(IProperties * ctxProps) const override { - bool success = CSpan::getSpanContext(ctxProps, otelFormatted); + CSpan::getSpanContext(ctxProps); - if (!otelFormatted && remoteParentSpanCtx.IsValid()) + if (remoteParentSpanCtx.IsValid()) { StringBuffer remoteParentSpanID; char remoteParentSpanId[16] = {0}; @@ -963,8 +961,6 @@ class CServerSpan : public CSpan remoteParentSpanID.append(16, remoteParentSpanId); ctxProps->setProp("remoteParentSpanID", remoteParentSpanID.str()); } - - return success; } void init(SpanFlags flags) @@ -1046,14 +1042,14 @@ class CServerSpan : public CSpan IProperties * getClientHeaders(const ISpan * span) { Owned<IProperties> headers = createProperties(true); - span->getSpanContext(headers, true); // Return value is not helpful + span->getClientHeaders(headers); return headers.getClear(); } IProperties * getSpanContext(const ISpan * span) { Owned<IProperties> headers = createProperties(true); - span->getSpanContext(headers, false); + span->getSpanContext(headers); return headers.getClear(); } diff --git a/system/jlib/jtrace.hpp b/system/jlib/jtrace.hpp index 6e0b34137e0..e0053684b55 100644 --- a/system/jlib/jtrace.hpp +++ b/system/jlib/jtrace.hpp @@ -61,7 +61,8 @@ interface ISpan : extends IInterface virtual void setSpanAttributes(const IProperties * attributes) = 0; virtual void addSpanEvent(const char * eventName) = 0; virtual void addSpanEvent(const char * eventName, IProperties * attributes) = 0; - virtual bool getSpanContext(IProperties * ctxProps, bool otelFormatted) const = 0; + virtual void getSpanContext(IProperties * ctxProps) const = 0; + virtual void getClientHeaders(IProperties * clientHeaders) const = 0; virtual void toString(StringBuffer & out) const = 0; virtual void getLogPrefix(StringBuffer & out) const = 0; diff --git a/testing/unittests/jlibtests.cpp b/testing/unittests/jlibtests.cpp index 862eb0d75e5..5cd207b9d20 100644 --- a/testing/unittests/jlibtests.cpp +++ b/testing/unittests/jlibtests.cpp @@ -259,9 +259,7 @@ class JlibTraceTest : public CppUnit::TestFixture Owned<ISpan> serverSpan = queryTraceManager().createServerSpan("noRemoteParentEnsureTraceID", emptyMockHTTPHeaders, flags); Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes.get(), false); - - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected - EnsureTraceID flag was set", true, getSpanCtxSuccess); + serverSpan->getSpanContext(retrievedSpanCtxAttributes.get()); CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected empty TraceID detected", false, isEmptyString(retrievedSpanCtxAttributes->queryProp("traceID"))); CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected empty SpanID detected", false, isEmptyString(retrievedSpanCtxAttributes->queryProp("spanID"))); @@ -279,9 +277,7 @@ class JlibTraceTest : public CppUnit::TestFixture //retrieve serverSpan context with the intent to interrogate attributes { Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes.get(), false); - - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); + serverSpan->getSpanContext(retrievedSpanCtxAttributes.get()); CPPUNIT_ASSERT_MESSAGE("Unexpected GlobalID detected", strsame("IncomingUGID", retrievedSpanCtxAttributes->queryProp(kGlobalIdHttpHeaderName))); @@ -296,12 +292,12 @@ class JlibTraceTest : public CppUnit::TestFixture //retrieve serverSpan context with the intent to propagate it to a remote child span { - Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes.get(), true); + Owned<IProperties> retrievedClientHeaders = createProperties(); + serverSpan->getClientHeaders(retrievedClientHeaders.get()); - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); + //CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getClientHeaders failure detected", true, retrievedClientHeaders->length()>0); CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected Otel traceparent header len detected", (size_t)55, - strlen(retrievedSpanCtxAttributes->queryProp("traceparent"))); + strlen(retrievedClientHeaders->queryProp("traceparent"))); } } @@ -326,7 +322,7 @@ class JlibTraceTest : public CppUnit::TestFixture { Owned<IProperties> headers = createProperties(true); - nullSpan->getSpanContext(headers, true); + nullSpan->getClientHeaders(headers); } Owned<ISpan> nullSpanChild = nullSpan->createClientSpan("nullSpanChild"); @@ -339,8 +335,7 @@ class JlibTraceTest : public CppUnit::TestFixture Owned<ISpan> serverSpan = queryTraceManager().createServerSpan("propegatedServerSpan", emptyMockHTTPHeaders); Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes, false); - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); + serverSpan->getSpanContext(retrievedSpanCtxAttributes); CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected empty spanID detected", true, retrievedSpanCtxAttributes->hasProp("spanID")); const char * serverSpanID = retrievedSpanCtxAttributes->queryProp("spanID"); @@ -353,8 +348,7 @@ class JlibTraceTest : public CppUnit::TestFixture //retrieve clientSpan context with the intent to propogate otel and HPCC context { Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = internalSpan->getSpanContext(retrievedSpanCtxAttributes, false); - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); + internalSpan->getSpanContext(retrievedSpanCtxAttributes); CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected missing localParentSpanID detected", true, retrievedSpanCtxAttributes->hasProp("localParentSpanID")); @@ -388,8 +382,7 @@ class JlibTraceTest : public CppUnit::TestFixture Owned<ISpan> serverSpan = queryTraceManager().createServerSpan("propegatedServerSpan", emptyMockHTTPHeaders); Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes, false); - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); + serverSpan->getSpanContext(retrievedSpanCtxAttributes); CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected empty spanID detected", true, retrievedSpanCtxAttributes->hasProp("spanID")); const char * serverSpanID = retrievedSpanCtxAttributes->queryProp("spanID"); @@ -402,8 +395,7 @@ class JlibTraceTest : public CppUnit::TestFixture //retrieve internalSpan context with the intent to interrogate attributes { Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = internalSpan->getSpanContext(retrievedSpanCtxAttributes, false); - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); + internalSpan->getSpanContext(retrievedSpanCtxAttributes); CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected missing localParentSpanID detected", true, retrievedSpanCtxAttributes->hasProp("localParentSpanID")); @@ -441,18 +433,15 @@ class JlibTraceTest : public CppUnit::TestFixture //retrieve serverSpan context with the intent to propagate it to a remote child span { - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes, true); + serverSpan->getClientHeaders(retrievedSpanCtxAttributes); - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected Otel traceparent header len detected", (size_t)55, strlen(retrievedSpanCtxAttributes->queryProp("traceparent"))); } //retrieve serverSpan context with the intent to interrogate attributes { - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes, false); - - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); + serverSpan->getSpanContext(retrievedSpanCtxAttributes); CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected GlobalID detected", false, retrievedSpanCtxAttributes->hasProp(kGlobalIdHttpHeaderName)); @@ -474,9 +463,8 @@ class JlibTraceTest : public CppUnit::TestFixture Owned<ISpan> serverSpan = queryTraceManager().createServerSpan("invalidPropegatedServerSpan", mockHTTPHeaders); Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes.get(), true); + serverSpan->getClientHeaders(retrievedSpanCtxAttributes.get()); - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); const char * traceParent = retrievedSpanCtxAttributes->queryProp("remoteParentSpanID"); DBGLOG("testInvalidPropegatedServerSpan: traceparent: %s", traceParent); } @@ -498,9 +486,7 @@ class JlibTraceTest : public CppUnit::TestFixture //remoteParentSpanID, globalID, callerID Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes.get(), false); - - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); + serverSpan->getSpanContext(retrievedSpanCtxAttributes.get()); CPPUNIT_ASSERT_MESSAGE("Unexpected GlobalID detected", strsame("IncomingUGID", retrievedSpanCtxAttributes->queryProp(kGlobalIdHttpHeaderName))); @@ -560,9 +546,7 @@ class JlibTraceTest : public CppUnit::TestFixture //retrieve serverSpan context with the intent to interrogate attributes { Owned<IProperties> retrievedClientSpanCtxAttributes = createProperties(); - bool getClientSpanCtxSuccess = clientSpan->getSpanContext(retrievedClientSpanCtxAttributes.get(), false); - - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected on client span", true, getClientSpanCtxSuccess); + clientSpan->getSpanContext(retrievedClientSpanCtxAttributes.get()); CPPUNIT_ASSERT_MESSAGE("Unexpected GlobalID detected", strsame("IncomingUGID", retrievedClientSpanCtxAttributes->queryProp(kGlobalIdHttpHeaderName))); @@ -586,9 +570,7 @@ class JlibTraceTest : public CppUnit::TestFixture //retrieve serverSpan context with the intent to interrogate attributes { Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes.get(), false); - - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); + serverSpan->getSpanContext(retrievedSpanCtxAttributes.get()); CPPUNIT_ASSERT_MESSAGE("Unexpected GlobalID detected", strsame("IncomingUGID", retrievedSpanCtxAttributes->queryProp(kGlobalIdHttpHeaderName))); @@ -605,9 +587,8 @@ class JlibTraceTest : public CppUnit::TestFixture //retrieve serverSpan context with the intent to propagate it to a remote child span { Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes.get(), true); + serverSpan->getClientHeaders(retrievedSpanCtxAttributes.get()); - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected Otel traceparent header len detected", (size_t)55, strlen(retrievedSpanCtxAttributes->queryProp("traceparent"))); } @@ -630,11 +611,7 @@ class JlibTraceTest : public CppUnit::TestFixture //retrieve serverSpan context with the intent to interrogate attributes { Owned<IProperties> retrievedSpanCtxAttributes = createProperties(); - bool getSpanCtxSuccess = serverSpan->getSpanContext(retrievedSpanCtxAttributes.get(), false); - - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); - - CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected getSpanContext failure detected", true, getSpanCtxSuccess); + serverSpan->getSpanContext(retrievedSpanCtxAttributes.get()); CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected GlobalID detected", true, strsame("someGlobalID", retrievedSpanCtxAttributes->queryProp(kGlobalIdHttpHeaderName)));