Skip to content

Avoid getTag overhead in ConflatingMetricsAggregator #10070

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

Merged
amarziali merged 1 commit into from
Dec 3, 2025
Merged

Avoid getTag overhead in ConflatingMetricsAggregator #10070

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
27 changes: 14 additions & 13 deletions dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import javax.annotation.Nonnull;
import org.jctools.queues.MpscCompoundQueue;
import org.jctools.queues.SpmcArrayQueue;
import org.slf4j.Logger;
Expand Down Expand Up @@ -276,15 +277,16 @@ public boolean publish(List<? extends CoreSpan<?>> trace) {
if (features.supportsMetrics()) {
for (CoreSpan<?> span : trace) {
boolean isTopLevel = span.isTopLevel();
if (shouldComputeMetric(span)) {
final CharSequence spanKind = span.unsafeGetTag(SPAN_KIND, "");
if (shouldComputeMetric(span, spanKind)) {
final CharSequence resourceName = span.getResourceName();
if (resourceName != null && ignoredResources.contains(resourceName.toString())) {
// skip publishing all children
forceKeep = false;
break;
}
counted++;
forceKeep |= publish(span, isTopLevel);
forceKeep |= publish(span, isTopLevel, spanKind);
}
}
healthMetrics.onClientStatTraceComputed(
Expand All @@ -293,21 +295,19 @@ public boolean publish(List<? extends CoreSpan<?>> trace) {
return forceKeep;
}

private boolean shouldComputeMetric(CoreSpan<?> span) {
return (span.isMeasured() || span.isTopLevel() || spanKindEligible(span))
private boolean shouldComputeMetric(CoreSpan<?> span, @Nonnull CharSequence spanKind) {
return (span.isMeasured() || span.isTopLevel() || spanKindEligible(spanKind))
&& span.getLongRunningVersion()
<= 0 // either not long-running or unpublished long-running span
&& span.getDurationNano() > 0;
}

private boolean spanKindEligible(CoreSpan<?> span) {
final Object spanKind = span.getTag(SPAN_KIND);
private boolean spanKindEligible(@Nonnull CharSequence spanKind) {
// use toString since it could be a CharSequence...
return spanKind != null && ELIGIBLE_SPAN_KINDS_FOR_METRICS.contains(spanKind.toString());
return ELIGIBLE_SPAN_KINDS_FOR_METRICS.contains(spanKind.toString());
}

private boolean publish(CoreSpan<?> span, boolean isTopLevel) {
final CharSequence spanKind = span.getTag(SPAN_KIND, "");
private boolean publish(CoreSpan<?> span, boolean isTopLevel, CharSequence spanKind) {
MetricKey newKey =
new MetricKey(
span.getResourceName(),
Expand Down Expand Up @@ -353,9 +353,10 @@ private boolean publish(CoreSpan<?> span, boolean isTopLevel) {

private List<UTF8BytesString> getPeerTags(CoreSpan<?> span, String spanKind) {
if (ELIGIBLE_SPAN_KINDS_FOR_PEER_AGGREGATION.contains(spanKind)) {
List<UTF8BytesString> peerTags = new ArrayList<>();
for (String peerTag : features.peerTags()) {
Object value = span.getTag(peerTag);
final Set<String> eligiblePeerTags = features.peerTags();
Copy link
Contributor

@dougqh dougqh Dec 5, 2025

Choose a reason for hiding this comment

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

As an aside, I've made a TagSet that might be better in this situations.
Rather than using a HashSet, it uses a flat array with linear probing.
That makes iteration faster

List<UTF8BytesString> peerTags = new ArrayList<>(eligiblePeerTags.size());
Copy link
Contributor

@bric3 bric3 Dec 3, 2025

Choose a reason for hiding this comment

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

praise: Good idea to pre-size the collection

for (String peerTag : eligiblePeerTags) {
Object value = span.unsafeGetTag(peerTag);
if (value != null) {
final Pair<DDCache<String, UTF8BytesString>, Function<String, UTF8BytesString>>
cacheAndCreator = PEER_TAGS_CACHE.computeIfAbsent(peerTag, PEER_TAGS_CACHE_ADDER);
Expand All @@ -368,7 +369,7 @@ private List<UTF8BytesString> getPeerTags(CoreSpan<?> span, String spanKind) {
return peerTags;
} else if (SPAN_KIND_INTERNAL.equals(spanKind)) {
// in this case only the base service should be aggregated if present
final Object baseService = span.getTag(BASE_SERVICE);
final Object baseService = span.unsafeGetTag(BASE_SERVICE);
if (baseService != null) {
final Pair<DDCache<String, UTF8BytesString>, Function<String, UTF8BytesString>>
cacheAndCreator = PEER_TAGS_CACHE.computeIfAbsent(BASE_SERVICE, PEER_TAGS_CACHE_ADDER);
Expand Down
8 changes: 8 additions & 0 deletions dd-trace-core/src/main/java/datadog/trace/core/CoreSpan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ public interface CoreSpan<T extends CoreSpan<T>> {

<U> U getTag(CharSequence name);

default <U> U unsafeGetTag(CharSequence name, U defaultValue) {
return getTag(name, defaultValue);
}

default <U> U unsafeGetTag(CharSequence name) {
return getTag(name);
}

boolean hasSamplingPriority();

boolean isMeasured();
Expand Down
13 changes: 13 additions & 0 deletions dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,19 @@ public Object getTag(final String tag) {
return context.getTag(tag);
}

@Override
@SuppressWarnings("unchecked")
public <U> U unsafeGetTag(CharSequence name, U defaultValue) {
Object tag = unsafeGetTag(name);
Copy link
Contributor

@dougqh dougqh Dec 5, 2025

Choose a reason for hiding this comment

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

Why not use getOrDefault on the Map?
I think that would be better because that's something that we can optimize on TagMap.

Copy link
Contributor Author

@amarziali amarziali Dec 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

good call I'll open a follow-up PR to adjust that. The main reason was that the method is not exposed on spanContext

return null == tag ? defaultValue : (U) tag;
}

@Override
@SuppressWarnings("unchecked")
public <U> U unsafeGetTag(CharSequence name) {
return (U) context.unsafeGetTag(String.valueOf(name));
}

@Override
@Nonnull
public final DDSpanContext context() {
Expand Down
Loading