Repository navigation
Tag database client spans with the identity encoded in RDS endpoint hostnames - #12599
Open
pedramsafaei wants to merge 8 commits into
Open
pedramsafaei wants to merge 8 commits into
pedramsafaei wants to merge 8 commits into
Conversation
pedramsafaei
marked this pull request as ready for review
September 23, 2026 16:03
pedramsafaei
requested review from
dougqh and
mcculls
and removed request for
a team
September 23, 2026 16:03
mcculls
reviewed
Sep 24, 2026
mcculls
reviewed
Sep 24, 2026
mcculls
reviewed
Sep 24, 2026
pedramsafaei
force-pushed
the
pedramsafaei/rds-endpoint-identity-tags
branch
from
September 24, 2026 17:25
2283a78 to
d74e127
Compare
…ostnames Database client spans against Amazon RDS only carried peer.hostname, for example orders-db.c9akciq32bzq.us-east-1.rds.amazonaws.com. That name deterministically encodes the DB instance (or Aurora cluster, custom endpoint or RDS Proxy) identifier and the Region, but nothing exposed them as queryable tags. Correlating a slow query with the aws.rds.* integration metrics for the same instance meant parsing the hostname by hand. DatabaseClientDecorator now parses the hostname whenever it matches an RDS endpoint and tags the span with: - aws.rds.identifier: the leading label (instance, cluster, custom endpoint or proxy name) - aws.rds.endpoint_type: instance, cluster, cluster-ro, cluster-custom or proxy - aws.region: the Region encoded in the hostname - dbinstanceidentifier when the endpoint is an instance endpoint, and dbclusteridentifier when it is a cluster or cluster reader endpoint. These match the dimension tags on the aws.rds.* metrics so APM spans and the RDS integration (and Database Monitoring) join on the same key. The instance or cluster identifier is only claimed when the endpoint type proves it. A cluster endpoint does not name the writer instance behind it, and a custom endpoint or proxy name is not a cluster identifier, so those types get aws.rds.identifier only. Both the standard (<id>.<hash>.<region>.rds.amazonaws.com) and China (<id>.<hash>.rds.<region>.amazonaws.com.cn) shapes are handled. Parsing is cached per hostname; hostnames not ending in ".amazonaws.com" or ".amazonaws.com.cn" (an O(1) suffix check) are rejected before the cache so they neither evict RDS entries nor pay for the parse, and are otherwise left untouched. So every existing database client integration (JDBC, R2DBC, Vert.x, ...) picks this up with no behaviour change outside RDS.
pedramsafaei
force-pushed
the
pedramsafaei/rds-endpoint-identity-tags
branch
from
September 24, 2026 19:54
d74e127 to
afcfb6a
Compare
mcculls
reviewed
Sep 24, 2026
mcculls
approved these changes
Sep 24, 2026
Contributor
There was a problem hiding this comment.
+1 from apm-sdk-capabilities-java
I did some rough benchmarking of master vs PR #12599 (commit afcfb6a), 3 forks × 15 iterations (30 samples/hostname)
┌──────────────────────────────────────────────────┬───────────────┬───────────────┬────────────┐
│ hostname │ master │ PR head │ added cost │
├──────────────────────────────────────────────────┼───────────────┼───────────────┼────────────┤
│ localhost │ 0.001 ± 0.001 │ 0.003 ± 0.001 │ +0.002 │
├──────────────────────────────────────────────────┼───────────────┼───────────────┼────────────┤
│ 127.0.0.1 │ 0.001 ± 0.001 │ 0.005 ± 0.001 │ +0.004 │
├──────────────────────────────────────────────────┼───────────────┼───────────────┼────────────┤
│ db.internal.corp │ 0.001 ± 0.001 │ 0.006 ± 0.001 │ +0.005 │
├──────────────────────────────────────────────────┼───────────────┼───────────────┼────────────┤
│ postgres.default.svc.cluster.local │ 0.001 ± 0.001 │ 0.008 ± 0.001 │ +0.007 │
├──────────────────────────────────────────────────┼───────────────┼───────────────┼────────────┤
│ orders-db...rds.amazonaws.com (RDS instance) │ 0.001 ± 0.001 │ 0.014 ± 0.001 │ +0.013 │
├──────────────────────────────────────────────────┼───────────────┼───────────────┼────────────┤
│ orders-cluster...rds.amazonaws.com (RDS cluster) │ 0.001 ± 0.001 │ 0.014 ± 0.001 │ +0.013 │
├──────────────────────────────────────────────────┼───────────────┼───────────────┼────────────┤
│ orders-db...rds.amazonaws.com.spoofed.example │ 0.001 ± 0.001 │ 0.006 ± 0.001 │ +0.005 │
├──────────────────────────────────────────────────┼───────────────┼───────────────┼────────────┤
│ my-instance...rds.cn-north-1.amazonaws.com.cn │ 0.001 ± 0.001 │ 0.015 ± 0.001 │ +0.014 │
└──────────────────────────────────────────────────┴───────────────┴───────────────┴────────────┘
Summary: the PR adds a small, bounded per-connection cost (2–7ns for non-RDS hosts, ~13–15ns for genuine RDS hosts due to the extra tagging etc.), negligible next to typical span overhead.
And that last length short-circuit pays off for small hostnames which are common in dev/test environments and local-first setups:
┌──────────────────────────────────────────────────┬──────────────────────┬───────────────────┬────────┐
│ hostname │ without length check │ with length check │ delta │
├──────────────────────────────────────────────────┼──────────────────────┼───────────────────┼────────┤
│ localhost (9 chars) │ 0.003 │ 0.001 │ −0.002 │
├──────────────────────────────────────────────────┼──────────────────────┼───────────────────┼────────┤
│ 127.0.0.1 (9 chars) │ 0.004 │ 0.001 │ −0.003 │
├──────────────────────────────────────────────────┼──────────────────────┼───────────────────┼────────┤
│ db.internal.corp (17 chars) │ 0.006 │ 0.006 │ 0 │
├──────────────────────────────────────────────────┼──────────────────────┼───────────────────┼────────┤
│ postgres.default.svc.cluster.local (35 chars) │ 0.008 │ 0.007 │ −0.001 │
├──────────────────────────────────────────────────┼──────────────────────┼───────────────────┼────────┤
│ orders-db...rds.amazonaws.com (RDS instance) │ 0.013 │ 0.013 │ 0 │
├──────────────────────────────────────────────────┼──────────────────────┼───────────────────┼────────┤
│ orders-cluster...rds.amazonaws.com (RDS cluster) │ 0.013 │ 0.013 │ 0 │
├──────────────────────────────────────────────────┼──────────────────────┼───────────────────┼────────┤
│ ...amazonaws.com.spoofed.example (lookalike) │ 0.006 │ 0.006 │ 0 │
├──────────────────────────────────────────────────┼──────────────────────┼───────────────────┼────────┤
│ China RDS │ 0.015 │ 0.015 │ 0 │
└──────────────────────────────────────────────────┴──────────────────────┴───────────────────┴────────┘
…strap/instrumentation/decorator/AwsRdsEndpoint.java Co-authored-by: Stuart McCulloch <mcculls@gmail.com>
Author
|
@dougqh quick bump here. Thank you |
The pre-cache gate strips only a well-formed :port (at most five ASCII digits), so it rejects a hostname with a malformed or oversized port even though the parser would truncate at the first ':' and could otherwise accept the labels before it. The previous Javadoc claimed the gate and the parser never disagree on what is RDS-shaped, which overstated the relationship. Describe the gate as a conservative pre-filter that only ever keeps a superset of non-RDS hosts out of the cache and never a valid RDS endpoint.
![datadog-prod-us1-3[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
Valid China, additional RDS Proxy, and ISO endpoint hostnames are rejected, leaving their database spans without the new RDS identity or Region tags.
🤖 Bits Code Review · Commit 7c23b21
| String[] labels = hostname.substring(0, end).split("\\.", -1); | ||
|
|
||
| // <identifier>.<hash>.<region>.rds.amazonaws.com | ||
| if (labels.length == 6 |
There was a problem hiding this comment.
Valid China endpoints such as orders-db.c9akciq32bzq.cn-north-1.rds.amazonaws.com.cn pass the suffix gate but fail both parsing branches: this branch requires six labels, while the China branch requires rds before the Region. These connections receive none of the new identity or Region tags. Accept both China layouts.
Suggested change
| if (labels.length == 6 | |
| if ((labels.length == 6 || (labels.length == 7 && "cn".equals(labels[6]))) |
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
| if (colon >= 0) { | ||
| end = Math.min(end, colon); | ||
| } | ||
| String[] labels = hostname.substring(0, end).split("\\.", -1); |
There was a problem hiding this comment.
Additional RDS Proxy endpoints use names such as orders-ro.endpoint.proxy-c9akciq32bzq.us-east-1.rds.amazonaws.com. The extra endpoint label prevents either parsing branch from matching, so applications using these endpoints, including reader endpoints, receive no proxy identity or Region tags. Normalize that label before the existing validated parsing.
Suggested change
| String[] labels = hostname.substring(0, end).split("\\.", -1); | |
| String[] labels = hostname.substring(0, end).split("\\.", -1); | |
| if (labels.length > 3 | |
| && "endpoint".equals(labels[1]) | |
| && labels[2].startsWith("proxy-")) { | |
| String[] normalizedLabels = new String[labels.length - 1]; | |
| normalizedLabels[0] = labels[0]; | |
| System.arraycopy(labels, 2, normalizedLabels, 1, labels.length - 2); | |
| labels = normalizedLabels; | |
| } |
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
| end--; | ||
| } | ||
| end = stripPort(hostname, end); | ||
| return endsWithIgnoreCase(hostname, end, AMAZONAWS_COM) |
There was a problem hiding this comment.
ISO partitions use DNS suffixes such as .c2s.ic.gov and .sc2s.sgov.gov. This gate rejects their RDS endpoints before parsing, omitting every new identity tag. The ISO fixture uses .amazonaws.com and therefore misses the failure. Both the suffix gate and parser need partition-aware handling; changing this gate alone is insufficient.
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
A user-defined RDS Proxy endpoint has the shape <name>.endpoint.proxy-<hash>.<region>.rds.amazonaws.com, with an extra endpoint label the default proxy endpoint does not carry. The parser only recognized the default shape, so read-only and other user-defined proxy endpoints matched neither branch and their database spans received none of the new identity or Region tags. Drop the endpoint label when the following label starts with proxy- so the proxy name and Region are read by the same validated branches as the default proxy endpoint, in both the standard and China layouts. The normalization is gated on the proxy- prefix, so a non-proxy endpoint label is still rejected.
Resolve additive conflicts in InstrumentationTags (new RDS identity tag constants alongside new AWS resource ARN constants) and in the database client decorator test imports.
Strip the partition DNS suffix (amazonaws.com, amazonaws.com.cn, c2s.ic.gov, sc2s.sgov.gov, from the AWS SDK endpoint partition data) before reading the labels, so the same parse serves commercial, GovCloud, China and the ISO partitions. Accept both the region-before-service layout <id>.<hash>.<region>.rds and the China service-before-region layout <id>.<hash>.rds.<region>, so a region-before-service China endpoint such as orders-db.<hash>.cn-north-1.rds.amazonaws.com.cn is now tagged. The pre-cache suffix gate accepts the same set of suffixes. A user-defined RDS Proxy endpoint keeps the existing endpoint-label normalization, now exercised in the ISO partition too, and the extra 'endpoint' label is only dropped for a real proxy hash so non-proxy hosts are not misclassified. Register the RDS identity tags on the db.client span type in the tag registry. Tests: add region-before-service China, ISO c2s.ic.gov and sc2s.sgov.gov instance/cluster/proxy cases, a user-defined proxy endpoint in the ISO partition, and negative cases for suffix lookalikes with appended labels and for an 'endpoint' label that is not a proxy.
There was a problem hiding this comment.
The suffix allowlists reject RDS endpoints in ISO-E, ISO-F, and European Sovereign Cloud, leaving those database spans without the new identity tags.
🤖 Bits Code Review · Commit 0acd850
Comment on lines
+91
to
+101
| private static final String[] DNS_SUFFIXES = { | ||
| ".amazonaws.com", ".amazonaws.com.cn", ".c2s.ic.gov", ".sc2s.sgov.gov" | ||
| }; | ||
| // The shortest suffix (".c2s.ic.gov"), used as a cheap length floor before any work. | ||
| private static final String SHORTEST_DNS_SUFFIX = ".c2s.ic.gov"; | ||
|
|
||
| // The same suffixes ordered longest first, so doParse strips ".amazonaws.com.cn" rather than | ||
| // shortening it to ".amazonaws.com" and misreading the labels. | ||
| private static final String[] LONGEST_FIRST_DNS_SUFFIXES = { | ||
| ".amazonaws.com.cn", ".sc2s.sgov.gov", ".amazonaws.com", ".c2s.ic.gov" | ||
| }; |
There was a problem hiding this comment.
RDS endpoints in ISO-E, ISO-F, and European Sovereign Cloud use .cloud.adc-e.uk, .csp.hci.ic.gov, and .amazonaws.eu. Both suffix lists omit these domains, so valid endpoints return null and their database spans receive none of the new identity tags. The eu-isoe-west-1 test uses amazonaws.com rather than the partition’s actual domain. Include the real domains in both lists and cover them in the parser tests.
Suggested change
| private static final String[] DNS_SUFFIXES = { | |
| ".amazonaws.com", ".amazonaws.com.cn", ".c2s.ic.gov", ".sc2s.sgov.gov" | |
| }; | |
| // The shortest suffix (".c2s.ic.gov"), used as a cheap length floor before any work. | |
| private static final String SHORTEST_DNS_SUFFIX = ".c2s.ic.gov"; | |
| // The same suffixes ordered longest first, so doParse strips ".amazonaws.com.cn" rather than | |
| // shortening it to ".amazonaws.com" and misreading the labels. | |
| private static final String[] LONGEST_FIRST_DNS_SUFFIXES = { | |
| ".amazonaws.com.cn", ".sc2s.sgov.gov", ".amazonaws.com", ".c2s.ic.gov" | |
| }; | |
| private static final String[] DNS_SUFFIXES = { | |
| ".amazonaws.com", | |
| ".amazonaws.com.cn", | |
| ".c2s.ic.gov", | |
| ".sc2s.sgov.gov", | |
| ".cloud.adc-e.uk", | |
| ".csp.hci.ic.gov", | |
| ".amazonaws.eu" | |
| }; | |
| // The shortest suffix (".c2s.ic.gov"), used as a cheap length floor before any work. | |
| private static final String SHORTEST_DNS_SUFFIX = ".c2s.ic.gov"; | |
| // The same suffixes ordered longest first, so doParse strips ".amazonaws.com.cn" rather than | |
| // shortening it to ".amazonaws.com" and misreading the labels. | |
| private static final String[] LONGEST_FIRST_DNS_SUFFIXES = { | |
| ".amazonaws.com.cn", | |
| ".cloud.adc-e.uk", | |
| ".csp.hci.ic.gov", | |
| ".sc2s.sgov.gov", | |
| ".amazonaws.com", | |
| ".amazonaws.eu", | |
| ".c2s.ic.gov" | |
| }; |
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
Recognize the published ISO-E, ISO-F, and European Sovereign Cloud DNS suffixes and test both parsing and pre-cache rejection behavior. Assisted-by: Kiro
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Does This Do
Database client spans against Amazon RDS currently carry only
peer.hostname, for exampleorders-db.c9akciq32bzq.us-east-1.rds.amazonaws.com. That hostname deterministically encodes the DB instance (or Aurora cluster, custom endpoint, or RDS Proxy) identifier and the Region, but nothing exposes them as tags.DatabaseClientDecorator.onConnectionnow parses the hostname whenever it matches an RDS endpoint and adds:aws.rds.identifieraws.rds.endpoint_typeinstance,cluster,cluster-ro,cluster-custom,proxyaws.regiondbinstanceidentifierinstanceendpoints onlydbclusteridentifierclusterandcluster-roendpoints onlydbinstanceidentifieranddbclusteridentifierare spelled exactly like the dimension tags on theaws.rds.*integration metrics, so an APM span and the RDS integration (and Database Monitoring) join on the same key without any hostname parsing on the query side.The instance or cluster identifier is only claimed when the endpoint type proves it. A cluster endpoint does not name the writer instance behind it, and a custom endpoint or proxy name is not a cluster identifier, so those types get
aws.rds.identifierandaws.rds.endpoint_typeonly.Implementation:
AwsRdsEndpointinagent-bootstrapparses both endpoint shapes,<id>.<hash>.<region>.rds.amazonaws.comand the China partition<id>.<hash>.rds.<region>.amazonaws.com.cn, tolerating a trailing dot or:port, case-insensitively. The hash label prefix (cluster-,cluster-ro-,cluster-custom-,proxy-, or none) determines the endpoint type. The hash and Region labels are validated so unrelated hostnames that happen to end inrds.amazonaws.comare rejected. Hostnames that do not end in.amazonaws.comor.amazonaws.com.cn(an O(1) case-insensitive suffix check that tolerates the same trailing.and:portas the parser) return before the cache, so non-AWS database hosts, and lookalikes with labels appended after.amazonaws.com, never evict RDS entries or pay for the label split. Plausible hostnames are then parsed once and cached in a 16-entryDDCache(applications talk to a handful of RDS databases).DatabaseClientDecoratorcalls it right after settingpeer.hostname, in a protectedonRdsEndpointhook. Because this is the shared base decorator, every database client integration (JDBC, R2DBC, Vert.x SQL, MongoDB, and so on) picks it up, and non-RDS hostnames are left exactly as before.InstrumentationTags.Motivation
Correlating a slow query span with the RDS instance it hit means matching
peer.hostnameagainst the instance endpoint by hand, and building any account- or Region-aware view of database dependencies means re-implementing the hostname parse in every dashboard or downstream consumer. The identity is already in the hostname; the tracer should surface it once, in the same vocabulary the RDS integration uses.A hostname cannot say which account owns the instance, so this intentionally stops at identifier, endpoint type and Region. Joining on
dbinstanceidentifierplusregionagainst theaws.rds.*metrics suppliesaws_account, which is why matching the metric dimension names exactly matters.Additional Notes
Testing
AwsRdsEndpointTest(new, JUnit 5, 47 cases): instance, cluster, cluster reader, custom and proxy endpoints; upper-case, trailing dot and:portinputs; GovCloud, ISO and China Regions; and 15 negative cases (non-RDS hosts, missing labels, bad hash, bad Region,s3/dynamodbservice endpoints, and a lookalike with a suffix appended). Also asserts the per-hostname cache returns the same instance; 19 rows for the pre-cache suffix gate (commercial and China with trailing dot and:port, upper and mixed case; rejects for non-AWS hosts,.amazonaws.comfollowed by further labels, and malformed ports) and one asserting 100 non-AWS and 100 suffix-lookalike hostnames do not evict a cached RDS endpoint.DatabaseClientDecoratorTest: newonConnectionrows asserting the exact tag set per endpoint type with strict interaction counting (0 * _), plus a non-RDS row asserting nothing is added.agent-bootstrapmodule: 686 tests, 0 failures.instrumentation/jdbcmodule: 423 tests, 0 failures (its fixtures use local hosts, confirming no behaviour change off RDS).spotlessApplyclean.How the end-to-end test was run
Publish, KinesisPutRecord, Step FunctionsStartExecution+DescribeExecution, LambdaInvoke, DynamoDBGetItem(by name and by ARN), S3GetObjectand a JDBCSELECT 1, all inside one traced method so the client spans share a trace.dd-java-agent.jarcopied into it: the released 1.66.0 tracer (before) and the jar built from this branch with./gradlew :dd-java-agent:shadowJar(after). Same node pool, sameDD_ENV, distinctDD_VERSIONso the two runs are separable in Span Search./api/v2/spans/events/search) was queried for each run's client spans and the per-span attributes compared key by key; the raw responses are retained. Where the change enables a join to the AWS integration metrics, the corresponding/api/v1/querywas run against the same org and the returned scope recorded.Live before/after on a real EKS workload
A Java 21 workload on EKS ran
SELECT 1through the PostgreSQL JDBC driver against a real RDS for PostgreSQL instance, once with stockdd-java-agent1.66.0 and once with the jar built from this branch (2283a78a22). Thepostgresql.queryspan as returned by the Span Search API:And the join it enables, from the same org's AWS integration metrics (account masked):
The same workload's AWS SDK spans (SNS, Kinesis, Step Functions, Lambda, DynamoDB) were unchanged between the two runs, as expected.
Design notes for reviewers
aws.regionis set, not a bareregiontag, to avoid colliding with the host-levelregiontag when a client in one Region talks to a database in another.dd.trace.db.client.rds.identity.enabledstyle flag if you prefer that for new tags.<id>.<hash>.<region>...), so an Aurora instance endpoint is reported asinstancewith its instance identifier, which is correct for thedbinstanceidentifierjoin.DatabaseClientDecorator.CACHEpattern. Only hostnames ending in.amazonaws.com/.amazonaws.com.cnreach it (review feedback), and a test asserts that distinct non-AWS and suffix-lookalike hosts do not evict a cached RDS entry.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labels — suggestedcomp: tracing,inst: jdbc,type: feature(external contributor, cannot set labels)close,fix, or any linking keywords when referencing an issuedd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/, already owned by@DataDog/apm-idm-javavia the existing directory entry, so no change neededSuggested labels (cannot set as an external contributor):
comp: tracing,inst: jdbc,type: feature.