Skip to content

[#noissue] Refine Pinpoint tracestate parsing #13882

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.

Sign up for GitHub

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

Open
emeroad wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[#noissue] Refine Pinpoint tracestate parsing #13882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 2 additions & 3 deletions .../src/main/java/com/navercorp/pinpoint/otlp/trace/collector/mapper/OtlpTraceConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,8 @@ public class OtlpTraceConstants {
// W3C tracestate vendor entry that carries upstream Pinpoint context. Conforms to other
// APM vendors' 2-letter key convention (dd, nr, dt, ot). Sub-keys inside the value use the
// OTel/Datadog style: ';' separates sub-keys, ':' separates sub-key name and value.
// Format: pp=svc:<parentServiceName>;app:<parentApplicationName>[;type:<serviceTypeCode>]
// The 'type' sub-key is optional; when absent the upstream is assumed to be another
// OTel-instrumented service and OPENTELEMETRY_SERVER is used as the parent service type.
// Format: pp=svc:<parentServiceName>;app:<parentApplicationName>;type:<serviceTypeCode>
// The 'svc', 'app', and 'type' sub-keys are required.
public static final String TRACESTATE_KEY_PINPOINT = "pp";
public static final String TRACESTATE_SUBKEY_PARENT_SERVICE_NAME = "svc";
public static final String TRACESTATE_SUBKEY_PARENT_APPLICATION_NAME = "app";
Expand Down
12 changes: 3 additions & 9 deletions ...src/main/java/com/navercorp/pinpoint/otlp/trace/collector/mapper/OtlpTraceSpanMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,7 @@ static void addTruncatedAnnotation(Consumer<AnnotationBo> sink, int truncatedAtt
* Applies an upstream Pinpoint context entry from {@code tracestate}.
* Mirrors the native {@code ServerRequestRecorder} behavior: parentServiceName is
* only meaningful when accompanied by a valid parentApplicationName, so the two
* fields are tied. The parent service type uses the {@code type} sub-key when
* present, otherwise falls back to {@link ServiceType#OPENTELEMETRY_SERVER} on the
* assumption that the upstream is another OTel-instrumented service. Invalid
* fields are tied. The parent service type uses the required {@code type} sub-key. Invalid
* applicationName (length / pattern) is silently dropped to avoid corrupting
* ApplicationMap row keys.
*/
Expand All @@ -217,14 +215,10 @@ static void addTruncatedAnnotation(Consumer<AnnotationBo> sink, int truncatedAtt
return null;
}
final String parentApplicationName = header.parentApplicationName();
if (parentApplicationName == null
|| !IdValidateUtils.validateId(parentApplicationName, PinpointConstants.APPLICATION_NAME_MAX_LEN_V3)) {
if (!IdValidateUtils.validateId(parentApplicationName, PinpointConstants.APPLICATION_NAME_MAX_LEN_V3)) {
return null;
}
final Integer parentApplicationType = header.parentApplicationType();
final int parentServiceType = parentApplicationType != null
? parentApplicationType
: ServiceType.OPENTELEMETRY_SERVER.getCode();
final int parentServiceType = header.parentApplicationType();
final String parentServiceName = header.parentServiceName();
return ParentApplication.of(parentServiceName, parentApplicationName, parentServiceType);
}
Expand Down
51 changes: 30 additions & 21 deletions ...ain/java/com/navercorp/pinpoint/otlp/trace/collector/mapper/PinpointTraceStateParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,54 +16,64 @@

package com.navercorp.pinpoint.otlp.trace.collector.mapper;

import org.jspecify.annotations.Nullable;

/**
* Extracts Pinpoint upstream context from the W3C {@code tracestate} header.
*
* <p>Expected entry: {@code pp=svc:<parentServiceName>;app:<parentApplicationName>[;type:<serviceTypeCode>]}.
* Any sub-key may be absent; unknown sub-keys are ignored so the format can be
* <p>Expected entry: {@code pp=svc:<parentServiceName>;app:<parentApplicationName>;type:<serviceTypeCode>}.
* All sub-keys are required; unknown sub-keys are ignored so the format can be
* extended later without breaking parsing.</p>
*/
final class PinpointTraceStateParser {

private static final String PINPOINT_ENTRY_PREFIX = OtlpTraceConstants.TRACESTATE_KEY_PINPOINT + "=";

private PinpointTraceStateParser() {
}

/** Parsed Pinpoint sub-keys from a {@code tracestate} header. Any field may be null. */
/** Parsed Pinpoint sub-keys from a {@code tracestate} header. */
record PinpointHeader(String parentServiceName,
String parentApplicationName,
Integer parentApplicationType) {
boolean isEmpty() {
return parentServiceName == null
&& parentApplicationName == null
&& parentApplicationType == null;
static @Nullable PinpointHeader ofNullable(@Nullable String parentServiceName,
@Nullable String parentApplicationName,
@Nullable Integer parentApplicationType) {
if (parentServiceName == null || parentApplicationName == null || parentApplicationType == null) {
return null;
}
return new PinpointHeader(parentServiceName, parentApplicationName, parentApplicationType);
}
}

/**
* @return parsed header or {@code null} when no usable {@code pp} entry exists
*/
static PinpointHeader parse(String traceState) {
static @Nullable PinpointHeader parse(@Nullable String traceState) {
if (traceState == null || traceState.isEmpty()) {
return null;
}
String pinpointEntryValue = findPinpointEntryValue(traceState, PINPOINT_ENTRY_PREFIX);
if (pinpointEntryValue == null) {
return null;
}
return parseValue(pinpointEntryValue);
}

private static @Nullable String findPinpointEntryValue(String traceState, String entryPrefix) {
for (String entry : traceState.split(",")) {
int eq = entry.indexOf('=');
if (eq < 0) {
continue;
}
String key = entry.substring(0, eq).trim();
if (!OtlpTraceConstants.TRACESTATE_KEY_PINPOINT.equals(key)) {
if (!entry.startsWith(entryPrefix)) {
continue;
}
// W3C tracestate top-level: on duplicate vendor keys, the first list-member
// wins. Sub-key duplicate semantics inside the value are vendor-defined;
// parseValue mirrors the same first-wins rule for consistency.
return parseValue(entry.substring(eq + 1).trim());
return entry.substring(entryPrefix.length());
}
return null;
}

private static PinpointHeader parseValue(String value) {
private static @Nullable PinpointHeader parseValue(String value) {
if (value.isEmpty()) {
return null;
}
Expand All @@ -75,8 +85,8 @@ private static PinpointHeader parseValue(String value) {
if (colon < 0) {
continue;
}
String subKey = sub.substring(0, colon).trim();
String subValue = sub.substring(colon + 1).trim();
String subKey = sub.substring(0, colon);
String subValue = sub.substring(colon + 1);
if (subValue.isEmpty()) {
continue;
}
Expand All @@ -95,12 +105,11 @@ private static PinpointHeader parseValue(String value) {
type = parseIntegerOrNull(subValue);
}
}
PinpointHeader header = new PinpointHeader(svc, app, type);
return header.isEmpty() ? null : header;
return PinpointHeader.ofNullable(svc, app, type);
}

/** Parse a Pinpoint ServiceType code; non-numeric or out-of-int-range returns null. */
private static Integer parseIntegerOrNull(String value) {
private static @Nullable Integer parseIntegerOrNull(String value) {
try {
return Integer.parseInt(value);
} catch (NumberFormatException e) {
Expand Down
41 changes: 16 additions & 25 deletions ...test/java/com/navercorp/pinpoint/otlp/trace/collector/mapper/OtlpTraceSpanMapperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import com.navercorp.pinpoint.common.server.bo.AnnotationBo;
import com.navercorp.pinpoint.common.server.bo.ParentApplication;
import com.navercorp.pinpoint.common.server.bo.SpanBo;
import com.navercorp.pinpoint.common.server.uid.ServiceUid;
import com.navercorp.pinpoint.common.trace.AnnotationKey;
import com.navercorp.pinpoint.common.trace.ServiceType;
import com.navercorp.pinpoint.common.trace.ServiceTypeFactory;
Expand Down Expand Up @@ -659,25 +658,23 @@ private static Span.Builder serverSpanBuilder() {
}

@Test
void map_tracestate_bothSubKeys_populatesParentFields() {
void map_tracestate_allSubKeys_populatesParentFields() {
Span span = serverSpanBuilder()
.setTraceState("pp=svc:upstream-svc;app:upstream-app")
.setTraceState("pp=svc:upstream-svc;app:upstream-app;type:1010")
.build();
SpanBo bo = newMapper().map(id(), span);
assertThat(bo.getParentApplication())
.isEqualTo(new ParentApplication("upstream-svc", "upstream-app",
(short) ServiceType.OPENTELEMETRY_SERVER.getCode()));
.isEqualTo(new ParentApplication("upstream-svc", "upstream-app", 1010));
}

@Test
void map_tracestate_multipleVendors_pickPinpoint() {
Span span = serverSpanBuilder()
.setTraceState("dd=s:1;t.dm:-4,pp=svc:upstream-svc;app:upstream-app,nr=opaque")
.setTraceState("dd=s:1;t.dm:-4,pp=svc:upstream-svc;app:upstream-app;type:1010,nr=opaque")
.build();
SpanBo bo = newMapper().map(id(), span);
assertThat(bo.getParentApplication())
.isEqualTo(new ParentApplication("upstream-svc", "upstream-app",
(short) ServiceType.OPENTELEMETRY_SERVER.getCode()));
.isEqualTo(new ParentApplication("upstream-svc", "upstream-app", 1010));
}

@Test
Expand All @@ -693,22 +690,20 @@ void map_tracestate_svcOnly_doesNotSetParentApplication() {
}

@Test
void map_tracestate_appOnly_setsApplicationWithDefaultService() {
void map_tracestate_appOnly_doesNotSetParentApplication() {
Span span = serverSpanBuilder()
.setTraceState("pp=app:upstream-app")
.build();
SpanBo bo = newMapper().map(id(), span);
assertThat(bo.getParentApplication())
.isEqualTo(new ParentApplication(ServiceUid.DEFAULT_SERVICE_UID_NAME, "upstream-app",
(short) ServiceType.OPENTELEMETRY_SERVER.getCode()));
assertThat(bo.getParentApplication()).isNull();
}

@Test
void map_tracestate_invalidApplicationName_silentlyDropped() {
// IdValidateUtils rejects non-ASCII; we silently drop rather than throw,
// to avoid corrupting ApplicationMap row keys.
Span span = serverSpanBuilder()
.setTraceState("pp=svc:upstream-svc;app:한글앱")
.setTraceState("pp=svc:upstream-svc;app:한글앱;type:1010")
.build();
SpanBo bo = newMapper().map(id(), span);
assertThat(bo.getParentApplication()).isNull();
Expand All @@ -730,7 +725,7 @@ void map_tracestate_trueRoot_skipsParentRecording() {
.setTraceId(ByteString.copyFrom(TRACE_ID))
.setSpanId(ByteString.copyFrom(SPAN_ID))
.setKindValue(Span.SpanKind.SPAN_KIND_SERVER_VALUE)
.setTraceState("pp=svc:upstream-svc;app:upstream-app")
.setTraceState("pp=svc:upstream-svc;app:upstream-app;type:1010")
.build();
SpanBo bo = newMapper().map(id(), span);
assertThat(bo.getParentApplication()).isNull();
Expand All @@ -747,12 +742,11 @@ void map_tracestate_consumerSpan_alsoApplies() {
.setKindValue(Span.SpanKind.SPAN_KIND_CONSUMER_VALUE)
.addAttributes(kv("messaging.system", strVal("kafka")))
.addAttributes(kv("messaging.destination.name", strVal("orders")))
.setTraceState("pp=svc:upstream-svc;app:upstream-app")
.setTraceState("pp=svc:upstream-svc;app:upstream-app;type:1010")
.build();
SpanBo bo = newMapper().map(id(), span);
assertThat(bo.getParentApplication())
.isEqualTo(new ParentApplication("upstream-svc", "upstream-app",
(short) ServiceType.OPENTELEMETRY_SERVER.getCode()));
.isEqualTo(new ParentApplication("upstream-svc", "upstream-app", 1010));
}

@Test
Expand All @@ -766,23 +760,20 @@ void map_tracestate_typeOverridesOtelServerDefault() {
}

@Test
void map_tracestate_typeMissing_fallsBackToOtelServer() {
void map_tracestate_typeMissing_doesNotSetParentApplication() {
Span span = serverSpanBuilder()
.setTraceState("pp=svc:upstream-svc;app:upstream-app")
.build();
SpanBo bo = newMapper().map(id(), span);
assertThat(bo.getParentApplication().applicationServiceType())
.isEqualTo(ServiceType.OPENTELEMETRY_SERVER.getCode());
assertThat(bo.getParentApplication()).isNull();
}

@Test
void map_tracestate_typeNonNumeric_fallsBackToOtelServer() {
void map_tracestate_typeNonNumeric_doesNotSetParentApplication() {
Span span = serverSpanBuilder()
.setTraceState("pp=app:upstream-app;type:tomcat")
.setTraceState("pp=svc:upstream-svc;app:upstream-app;type:tomcat")
.build();
SpanBo bo = newMapper().map(id(), span);
assertThat(bo.getParentApplication())
.isEqualTo(new ParentApplication(ServiceUid.DEFAULT_SERVICE_UID_NAME, "upstream-app",
ServiceType.OPENTELEMETRY_SERVER.getCode()));
assertThat(bo.getParentApplication()).isNull();
}
}
Loading
Loading