Skip to content

Reducing memory allocation from client-side stats #10705

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
dougqh wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Reducing memory allocation from client-side stats #10705

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b453f26
Reducing memory allocation from client-side stats
dougqh Feb 27, 2026
bd6a418
Addressing review feedback
dougqh Mar 9, 2026
123ed37
Fixing null handling
dougqh Mar 9, 2026
8f5255d
Cleaning up null handling - reducing diff size
dougqh Mar 9, 2026
eae7a89
Merge branch 'master' into dougqh/client-side-stats-overhead
dougqh Mar 9, 2026
419a462
Merge branch 'master' into dougqh/client-side-stats-overhead
dougqh Mar 10, 2026
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
37 changes: 29 additions & 8 deletions dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import static datadog.trace.bootstrap.instrumentation.api.UTF8BytesString.EMPTY;

import datadog.trace.api.cache.DDCache;
import datadog.trace.api.cache.DDCaches;
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
import datadog.trace.util.HashingUtils;
import java.util.Collections;
Expand All @@ -10,6 +12,17 @@

/** The aggregation key for tracked metrics. */
public final class MetricKey {
static final DDCache<String, UTF8BytesString> RESOURCE_CACHE = DDCaches.newFixedSizeCache(32);
static final DDCache<String, UTF8BytesString> SERVICE_CACHE = DDCaches.newFixedSizeCache(8);
static final DDCache<String, UTF8BytesString> SERVICE_SOURCE_CACHE =
Copy link
Contributor

@amarziali amarziali Mar 10, 2026

Choose a reason for hiding this comment

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

That field does not need to be cached, for two main reasons:

  1. It is already either a UTF8BytesString or null. In fact, it comes from the component() method, which in helpers typically returns a UTF8BytesString.
  2. Its cardinality matches that of the instrumentations, the size of the cache is too small.

The constructor calling UTF8BytesString.create() may look confusing at first glance, but in this case it does not allocate a new object — it simply returns the existing instance. It was kept for consistency with the usual pattern.

Unless there are strong counterarguments, I would suggest removing the cache for this field.

Copy link
Contributor Author

@dougqh dougqh Mar 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I think the key part here is "typically returns a UTF8BytesString".
The static typing doesn't currently guarantee it.

And the caching code accounts for the case of receiving a UTF8BytesString by bypassing the cache. In the same manner as UTF8BytesString.create did previously.

When a UTF8BytesString is passed, the cache incurs no overhead other than the static overhead of the cache array itself. But in the case where something other than UTF8BytesString is passed, the cache can significantly reduce memory consumption.

I think that's important because we need to curtail the worst case outcomes.

As for the size, the cardinality doesn't need to match total instrumentations. That would be too large. The size just needs to accommodate the active span producing instrumentations. As I said, I erred on the small size, since I want to avoid the worst case of consuming a lot of static memory.

And assuming a UTF8BytesString is introducing a subtle form of coupling that could easily be compromised by someone later on.

DDCaches.newFixedSizeCache(16);
static final DDCache<String, UTF8BytesString> OPERATION_CACHE = DDCaches.newFixedSizeCache(64);
static final DDCache<String, UTF8BytesString> TYPE_CACHE = DDCaches.newFixedSizeCache(8);
static final DDCache<String, UTF8BytesString> KIND_CACHE = DDCaches.newFixedSizeCache(8);
static final DDCache<String, UTF8BytesString> HTTP_METHOD_CACHE = DDCaches.newFixedSizeCache(8);
static final DDCache<String, UTF8BytesString> HTTP_ENDPOINT_CACHE =
DDCaches.newFixedSizeCache(32);

private final UTF8BytesString resource;
private final UTF8BytesString service;
private final UTF8BytesString serviceSource;
Expand Down Expand Up @@ -37,18 +50,18 @@ public MetricKey(
List<UTF8BytesString> peerTags,
CharSequence httpMethod,
CharSequence httpEndpoint) {
this.resource = null == resource ? EMPTY : UTF8BytesString.create(resource);
this.service = null == service ? EMPTY : UTF8BytesString.create(service);
this.serviceSource = null == serviceSource ? null : UTF8BytesString.create(serviceSource);
this.operationName = null == operationName ? EMPTY : UTF8BytesString.create(operationName);
this.type = null == type ? EMPTY : UTF8BytesString.create(type);
this.resource = null == resource ? EMPTY : utf8(RESOURCE_CACHE, resource);
this.service = null == service ? EMPTY : utf8(SERVICE_CACHE, service);
this.serviceSource = null == serviceSource ? null : utf8(SERVICE_SOURCE_CACHE, serviceSource);
this.operationName = null == operationName ? EMPTY : utf8(OPERATION_CACHE, operationName);
this.type = null == type ? EMPTY : utf8(TYPE_CACHE, type);
this.httpStatusCode = httpStatusCode;
this.synthetics = synthetics;
this.isTraceRoot = isTraceRoot;
this.spanKind = null == spanKind ? EMPTY : UTF8BytesString.create(spanKind);
this.spanKind = null == spanKind ? EMPTY : utf8(KIND_CACHE, spanKind);
this.peerTags = peerTags == null ? Collections.emptyList() : peerTags;
this.httpMethod = httpMethod == null ? null : UTF8BytesString.create(httpMethod);
this.httpEndpoint = httpEndpoint == null ? null : UTF8BytesString.create(httpEndpoint);
this.httpMethod = httpMethod == null ? null : utf8(HTTP_METHOD_CACHE, httpMethod);
this.httpEndpoint = httpEndpoint == null ? null : utf8(HTTP_ENDPOINT_CACHE, httpEndpoint);

int tmpHash = 0;
tmpHash = HashingUtils.addToHash(tmpHash, this.isTraceRoot);
Expand All @@ -66,6 +79,14 @@ public MetricKey(
this.hash = tmpHash;
}

static UTF8BytesString utf8(DDCache<String, UTF8BytesString> cache, CharSequence charSeq) {
if (charSeq instanceof UTF8BytesString) {
return (UTF8BytesString) charSeq;
} else {
return cache.computeIfAbsent(charSeq.toString(), UTF8BytesString::create);
}
}

public UTF8BytesString getResource() {
return resource;
}
Expand Down
Loading