feat(telemetry): cross-component async write tracing #12405
Merged
Conversation
This file contains 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
github-actions
bot
added
product
datahub-cyborg
bot
added
the
needs-review
david-leifker
force-pushed
the
api-tracing
branch
from
69db86b to
18495bb
Compare
david-leifker
force-pushed
the
api-tracing
branch
from
18495bb to
2589dfe
Compare
david-leifker
force-pushed
the
api-tracing
branch
from
2589dfe to
bc50082
Compare
* created TraceContext for opentelemetry spans * added tracing header/cookies to control logging trace info * support legacy dropwizard tracing using opentelemetry * added smoke-tests for tracing conditions
david-leifker
force-pushed
the
api-tracing
branch
from
bc50082 to
d101d78
Compare
RyanHolstien
approved these changes
Jan 28, 2025
There was a problem hiding this comment.
A lot of changes here so there was some quick scanning in review, but everything was looking good from what I saw. If there's any specific parts you were looking for more feedback on let me know and we can go through it, but very excited to get this in!
Probably worth a demo to at least platform team (maybe recorded for CS) on how to utilize it. Very cool stuff and nice work!
metadata-io/src/main/java/com/linkedin/metadata/service/UpdateIndicesService.java
Outdated
Show resolved
Hide resolved
metadata-io/src/main/java/com/linkedin/metadata/trace/KafkaTraceReader.java
Outdated
Show resolved
Hide resolved
| // Process records for each aspect name we haven't found yet | ||
| for (String aspectName : aspectNames) { | ||
| if (!results.containsKey(aspectName)) { | ||
| var matchingRecord = |
There was a problem hiding this comment.
We generally avoid var in the rest of the codebase so a bit weird to see it here, but I think it's fine since we don't need to run metadata-io in Java 11/8 compat
...mce-consumer/src/main/java/com/linkedin/metadata/kafka/MetadataChangeProposalsProcessor.java
Show resolved
Hide resolved
datahub-cyborg
bot
added
pending-submitter-merge
and removed
needs-review
|
Failing cypress is not reproduce-able locally. |
david-leifker
merged commit 412600a
into
datahub-project:master
195 of 200 checks passed
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.
Key Features:
Async write requests (OpenAPI/Rest.li) will include a
trace idgenerated by OpenTelemetry and returned via a standard http response header used for tracing:traceparent.SystemMetadata, available from OpenAPIv3, will also include thetrace idin the properties with keytelemetryTraceIdtrace idcan be used to track the outcome from a write request with information about its success/failure or pending status.trace idfrom GMS to the consumers (mce-consumerandmae-consumer).00062c2b698bcb28e92508f8f311802dtelemetryTraceIdvalue in the response.{ "urn": "urn:li:dataset:(urn:li:dataPlatform:bigquery,transactions.user_profile,PROD)", "status": { "value": { "removed": false }, "systemMetadata": { "properties": { "telemetryLog": "true", "telemetryQueueSpanId": "94b0303be3c8e91c", "telemetryTraceId": "00062c2b698bcb28e92508f8f311802d", "telemetryEnqueuedAt": "1737414826271" }, "lastRunId": "no-run-id-provided", "lastObserved": 1737414826266, "runId": "no-run-id-provided" }, "auditStamp": { "actor": "urn:li:corpuser:datahub", "time": 1737414826266 } } }trace idand the URN/aspect.{ "urn:li:dataset:(urn:li:dataPlatform:bigquery,transactions.user_profile,PROD)": { "status": { "success": true, "primaryStorage": { "writeStatus": "ACTIVE_STATE" }, "searchStorage": { "writeStatus": "ACTIVE_STATE" } } } }The Failed MCP topic will now store more detailed error messages and the trace API will fetch these errors in order to not only return failure status, but detailed information on why it failed.
For debugging, a cookie or special header, can be used with any request (read/write/sync/async) using any API (Graphql/OpenAPI/Rest.li) and will trigger logging of the spans with detailed timing of the request in the logs.
X-Enable-Trace-Logwith valuetrueenable-trace-logwith valuetrueDesign Considerations:
For the initial implementation no specific telemetry infrastructure is required, however existing environment variables for OpenTelemetry can continue to be used and will export the new spans if configured.
trace idor related timestamps.trace idis stored insystemMetadatain both SQL and ES. For ES specifically, the presence of thetrace idin the system metadata index is used as a proxy to determine a successful write to ES.Trace performance
skipCacheis included as a flag to bypass the cache.This PR updates OpenTelemetry and transitions from the DropWizard based timing instrumentation to using OpenTelemetry. The existing metrics for DropWizard are forwarded from OpenTelemetry preserving the existing naming scheme.
For easy access, the
OperationContextnow includes aTraceContextto facilitate the integration of OpenTelemetry into any part of the code base.Checklist