Skip to content

Implement span started and live health metrics #7430

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.

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
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
Comparing source compatibility of opentelemetry-sdk-trace-1.52.0-SNAPSHOT.jar against opentelemetry-sdk-trace-1.51.0.jar
No changes.
*** MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.sdk.trace.SdkTracerProviderBuilder (not serializable)
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.trace.SdkTracerProviderBuilder setInternalTelemetry(io.opentelemetry.sdk.common.InternalTelemetryVersion)
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.trace.SdkTracerProviderBuilder setMeterProvider(java.util.function.Supplier<io.opentelemetry.api.metrics.MeterProvider>)
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ public String toString(boolean includePrefixAndSuffix) {
joiner.add("executorService=" + executorService);
}
joiner.add("exporterType=" + exporterType.toString());
joiner.add("internalTelemetrySchemaVersion=" + internalTelemetryVersion);
joiner.add("internalTelemetryVersion=" + internalTelemetryVersion);
// Note: omit tlsConfigHelper because we can't log the configuration in any readable way
// Note: omit meterProviderSupplier because we can't log the configuration in any readable way
return joiner.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ public String toString(boolean includePrefixAndSuffix) {
joiner.add("executorService=" + executorService);
}
joiner.add("exporterType=" + exporterType);
joiner.add("internalTelemetrySchemaVersion=" + internalTelemetryVersion);
joiner.add("internalTelemetryVersion=" + internalTelemetryVersion);
// Note: omit tlsConfigHelper because we can't log the configuration in any readable way
// Note: omit meterProviderSupplier because we can't log the configuration in any readable way
return joiner.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ String toString(boolean includePrefixAndSuffix) {
joiner.add("endpoint=" + endpoint);
joiner.add("compressionEnabled=" + compressionEnabled);
joiner.add("readTimeoutMillis=" + readTimeoutMillis);
joiner.add("internalTelemetrySchemaVersion=" + internalTelemetryVersion);
joiner.add("internalTelemetryVersion=" + internalTelemetryVersion);
// Note: omit sender because we can't log the configuration in any readable way
// Note: omit encoder because we can't log the configuration in any readable way
// Note: omit localIpAddressSupplier because we can't log the configuration in any readable way
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ void stringRepresentation() {
try (ZipkinSpanExporter exporter = ZipkinSpanExporter.builder().build()) {
assertThat(exporter.toString())
.isEqualTo(
"ZipkinSpanExporter{endpoint=http://localhost:9411/api/v2/spans, compressionEnabled=true, readTimeoutMillis=10000, internalTelemetrySchemaVersion=LEGACY}");
"ZipkinSpanExporter{endpoint=http://localhost:9411/api/v2/spans, compressionEnabled=true, readTimeoutMillis=10000, internalTelemetryVersion=LEGACY}");
}
try (ZipkinSpanExporter exporter =
ZipkinSpanExporter.builder()
Expand All @@ -266,7 +266,7 @@ void stringRepresentation() {
.build()) {
assertThat(exporter.toString())
.isEqualTo(
"ZipkinSpanExporter{endpoint=http://zipkin:9411/api/v2/spans, compressionEnabled=false, readTimeoutMillis=15000, internalTelemetrySchemaVersion=LEGACY}");
"ZipkinSpanExporter{endpoint=http://zipkin:9411/api/v2/spans, compressionEnabled=false, readTimeoutMillis=15000, internalTelemetryVersion=LEGACY}");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ private SemConvAttributes() {}
AttributeKey.stringKey("otel.component.name");
public static final AttributeKey<String> ERROR_TYPE = AttributeKey.stringKey("error.type");

public static final AttributeKey<String> OTEL_SPAN_SAMPLING_RESULT =
AttributeKey.stringKey("otel.span.sampling_result");

public static final AttributeKey<String> SERVER_ADDRESS =
AttributeKey.stringKey("server.address");
public static final AttributeKey<Long> SERVER_PORT = AttributeKey.longKey("server.port");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ void testAttributeKeys() {
.isEqualTo(OtelIncubatingAttributes.OTEL_COMPONENT_NAME);
assertThat(SemConvAttributes.OTEL_COMPONENT_TYPE)
.isEqualTo(OtelIncubatingAttributes.OTEL_COMPONENT_TYPE);
assertThat(SemConvAttributes.OTEL_SPAN_SAMPLING_RESULT)
.isEqualTo(OtelIncubatingAttributes.OTEL_SPAN_SAMPLING_RESULT);

assertThat(SemConvAttributes.ERROR_TYPE).isEqualTo(ErrorAttributes.ERROR_TYPE);

Expand Down
1 change: 1 addition & 0 deletions sdk/trace/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ dependencies {
testImplementation(project(":sdk:testing"))
testImplementation("com.google.guava:guava")
testImplementation("com.google.guava:guava-testlib")
testImplementation("io.opentelemetry.semconv:opentelemetry-semconv-incubating")

jmh(project(":sdk:metrics"))
jmh(project(":sdk:testing")) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.trace;

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanContext;
import io.opentelemetry.api.trace.StatusCode;
import io.opentelemetry.sdk.trace.internal.metrics.SpanMetrics;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;

/**
* Span implementation used from {@link io.opentelemetry.sdk.trace.SdkTracer} when starting a span
* which is not recording. All operations are noop, except for {@link #end()}, which ensures health
* metrics are still collected.
*/
@Immutable
final class NonRecordingSpan implements Span {

private final SpanContext spanContext;
private final SpanMetrics.Recording metricRecording;

NonRecordingSpan(SpanContext spanContext, SpanMetrics.Recording metricRecording) {
this.spanContext = spanContext;
this.metricRecording = metricRecording;
}

@Override
public Span setAttribute(String key, @Nullable String value) {
return this;

Check warning on line 36 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L36

Added line #L36 was not covered by tests
}

@Override
public Span setAttribute(String key, long value) {
return this;

Check warning on line 41 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L41

Added line #L41 was not covered by tests
}

@Override
public Span setAttribute(String key, double value) {
return this;

Check warning on line 46 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L46

Added line #L46 was not covered by tests
}

@Override
public Span setAttribute(String key, boolean value) {
return this;

Check warning on line 51 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L51

Added line #L51 was not covered by tests
}

@Override
public <T> Span setAttribute(AttributeKey<T> key, @Nullable T value) {
return this;

Check warning on line 56 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L56

Added line #L56 was not covered by tests
}

@Override
public Span setAllAttributes(Attributes attributes) {
return this;

Check warning on line 61 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L61

Added line #L61 was not covered by tests
}

@Override
public Span addEvent(String name) {
return this;

Check warning on line 66 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L66

Added line #L66 was not covered by tests
}

@Override
public Span addEvent(String name, long timestamp, TimeUnit unit) {
return this;

Check warning on line 71 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L71

Added line #L71 was not covered by tests
}

@Override
public Span addEvent(String name, Attributes attributes) {
return this;

Check warning on line 76 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L76

Added line #L76 was not covered by tests
}

@Override
public Span addEvent(String name, Attributes attributes, long timestamp, TimeUnit unit) {
return this;

Check warning on line 81 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L81

Added line #L81 was not covered by tests
}

@Override
public Span setStatus(StatusCode statusCode) {
return this;

Check warning on line 86 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L86

Added line #L86 was not covered by tests
}

@Override
public Span setStatus(StatusCode statusCode, String description) {
return this;

Check warning on line 91 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L91

Added line #L91 was not covered by tests
}

@Override
public Span recordException(Throwable exception) {
return this;

Check warning on line 96 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L96

Added line #L96 was not covered by tests
}

@Override
public Span recordException(Throwable exception, Attributes additionalAttributes) {
return this;

Check warning on line 101 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L101

Added line #L101 was not covered by tests
}

@Override
public Span updateName(String name) {
return this;

Check warning on line 106 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L106

Added line #L106 was not covered by tests
}

@Override
public void end() {
metricRecording.recordSpanEnd();
}

@Override
public void end(long timestamp, TimeUnit unit) {
end();
}

Check warning on line 117 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L116-L117

Added lines #L116 - L117 were not covered by tests

@Override
public SpanContext getSpanContext() {
return spanContext;
}

@Override
public boolean isRecording() {
return false;
}

@Override
public String toString() {
return "NonRecordingSpan{" + spanContext + '}';

Check warning on line 131 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java#L131

Added line #L131 was not covered by tests
}
}
14 changes: 11 additions & 3 deletions sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.data.StatusData;
import io.opentelemetry.sdk.trace.internal.ExtendedSpanProcessor;
import io.opentelemetry.sdk.trace.internal.metrics.SpanMetrics;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -99,6 +100,8 @@ final class SdkSpan implements ReadWriteSpan {
@GuardedBy("lock")
private long endEpochNanos;

private final SpanMetrics.Recording metricRecording;

private enum EndState {
NOT_ENDED,
ENDING,
Expand Down Expand Up @@ -132,7 +135,8 @@ private SdkSpan(
@Nullable AttributesMap attributes,
@Nullable List<LinkData> links,
int totalRecordedLinks,
long startEpochNanos) {
long startEpochNanos,
SpanMetrics.Recording metricRecording) {
this.context = context;
this.instrumentationScopeInfo = instrumentationScopeInfo;
this.parentSpanContext = parentSpanContext;
Expand All @@ -143,6 +147,7 @@ private SdkSpan(
this.spanProcessor = spanProcessor;
this.exceptionAttributeResolver = exceptionAttributeResolver;
this.resource = resource;
this.metricRecording = metricRecording;
this.hasEnded = EndState.NOT_ENDED;
this.clock = clock;
this.startEpochNanos = startEpochNanos;
Expand Down Expand Up @@ -180,7 +185,8 @@ static SdkSpan startSpan(
@Nullable AttributesMap attributes,
@Nullable List<LinkData> links,
int totalRecordedLinks,
long userStartEpochNanos) {
long userStartEpochNanos,
SpanMetrics.Recording metricsRecording) {
boolean createdAnchoredClock;
AnchoredClock clock;
if (parentSpan instanceof SdkSpan) {
Expand Down Expand Up @@ -219,7 +225,8 @@ static SdkSpan startSpan(
attributes,
links,
totalRecordedLinks,
startEpochNanos);
startEpochNanos,
metricsRecording);
// Call onStart here instead of calling in the constructor to make sure the span is completely
// initialized.
if (spanProcessor.isStartRequired()) {
Expand Down Expand Up @@ -557,6 +564,7 @@ private void endInternal(long endEpochNanos) {
spanEndingThread = Thread.currentThread();
hasEnded = EndState.ENDING;
}
metricRecording.recordSpanEnd();
if (spanProcessor instanceof ExtendedSpanProcessor) {
ExtendedSpanProcessor extendedSpanProcessor = (ExtendedSpanProcessor) spanProcessor;
if (extendedSpanProcessor.isOnEndingRequired()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import io.opentelemetry.sdk.internal.AttributeUtil;
import io.opentelemetry.sdk.internal.AttributesMap;
import io.opentelemetry.sdk.trace.data.LinkData;
import io.opentelemetry.sdk.trace.internal.metrics.SpanMetrics;
import io.opentelemetry.sdk.trace.samplers.SamplingDecision;
import io.opentelemetry.sdk.trace.samplers.SamplingResult;
import java.util.ArrayList;
Expand Down Expand Up @@ -204,8 +205,10 @@ public Span startSpan() {
/* remote= */ false,
tracerSharedState.isIdGeneratorSafeToSkipIdValidation());

SpanMetrics.Recording metricsRecording =
tracerSharedState.getSpanMetrics().recordSpanStart(samplingResult);
if (!isRecording(samplingDecision)) {
return Span.wrap(spanContext);
return new NonRecordingSpan(spanContext, metricsRecording);
}
Attributes samplingAttributes = samplingResult.getAttributes();
if (!samplingAttributes.isEmpty()) {
Expand All @@ -232,7 +235,8 @@ public Span startSpan() {
recordedAttributes,
currentLinks,
totalNumberOfLinksAdded,
startEpochNanos);
startEpochNanos,
metricsRecording);
}

private AttributesMap attributes() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,22 @@

package io.opentelemetry.sdk.trace;

import io.opentelemetry.api.metrics.MeterProvider;
import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.api.trace.TracerBuilder;
import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.sdk.common.Clock;
import io.opentelemetry.sdk.common.CompletableResultCode;
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import io.opentelemetry.sdk.common.InternalTelemetryVersion;
import io.opentelemetry.sdk.internal.ComponentRegistry;
import io.opentelemetry.sdk.internal.ExceptionAttributeResolver;
import io.opentelemetry.sdk.internal.ScopeConfigurator;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.trace.internal.SdkTracerProviderUtil;
import io.opentelemetry.sdk.trace.internal.TracerConfig;
import io.opentelemetry.sdk.trace.internal.metrics.SemConvSpanMetrics;
import io.opentelemetry.sdk.trace.internal.metrics.SpanMetrics;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import java.io.Closeable;
import java.util.List;
Expand Down Expand Up @@ -54,7 +58,9 @@
Sampler sampler,
List<SpanProcessor> spanProcessors,
ScopeConfigurator<TracerConfig> tracerConfigurator,
ExceptionAttributeResolver exceptionAttributeResolver) {
ExceptionAttributeResolver exceptionAttributeResolver,
InternalTelemetryVersion internalTelemetryVersion,
Supplier<MeterProvider> meterProviderSupplier) {
this.sharedState =
new TracerSharedState(
clock,
Expand All @@ -63,7 +69,8 @@
spanLimitsSupplier,
sampler,
spanProcessors,
exceptionAttributeResolver);
exceptionAttributeResolver,
createSpanMetrics(internalTelemetryVersion, meterProviderSupplier));
this.tracerSdkComponentRegistry =
new ComponentRegistry<>(
instrumentationScopeInfo ->
Expand All @@ -74,6 +81,19 @@
this.tracerConfigurator = tracerConfigurator;
}

private static SpanMetrics createSpanMetrics(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's follow the pattern of ExporterInstrumentation:

  • Let's have a class called SpanInstrumentation (or maybe TracerProviderInstrumentation)
  • Class should have a constructor which accepts InternalTelemetryVersion and Supplier<MeterProvider> and initializes accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed in 6f4b0af.

However, I kept SpanInstrumentation as an interface instead of class with a static create method for now: It doesn't contain any logic except for create, which is why I think making it a class and adding delegation like ExporterInstrumentation would be unnecessary boilerplate. It however does allow us to change to this model later if we need it without touching the consumers of SpanInstrumentation.

InternalTelemetryVersion internalTelemetryVersion,
Supplier<MeterProvider> meterProviderSupplier) {
switch (internalTelemetryVersion) {
case LEGACY:
return SpanMetrics.noop();
case LATEST:
return new SemConvSpanMetrics(meterProviderSupplier);
}
throw new IllegalStateException(

Check warning on line 93 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java#L93

Added line #L93 was not covered by tests
"Unhandled telemetry schema version: " + internalTelemetryVersion);
}

private TracerConfig getTracerConfig(InstrumentationScopeInfo instrumentationScopeInfo) {
TracerConfig tracerConfig = tracerConfigurator.apply(instrumentationScopeInfo);
return tracerConfig == null ? TracerConfig.defaultConfig() : tracerConfig;
Expand Down
Loading
Loading