Create a better OperationName for Activity/OpenTelemetry
#4700
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| // <copyright file="OperationNameMapper.cs" company="Datadog"> | ||
| // Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. | ||
| // </copyright> | ||
|
|
||
| #nullable enable | ||
| using System; | ||
| using Datadog.Trace.Tagging; | ||
|
|
||
| namespace Datadog.Trace.Activity | ||
| { | ||
| /// <summary> | ||
| /// Helper class to map <see cref="SpanKinds"/> and various tags on an <c>Activity</c> or OpenTelemetry Span to a <see cref="Span.OperationName"/>. | ||
| /// </summary> | ||
| internal static class OperationNameMapper | ||
|
There was a problem hiding this comment. Really nice and declarative 👍 |
||
| { | ||
| internal static void MapToOperationName(Span span) | ||
| { | ||
| if (!string.IsNullOrEmpty(span.OperationName)) | ||
| { | ||
| return; // if OperationName has a value it means there was an "operation.name" tag | ||
| } | ||
|
|
||
| string operationName; | ||
|
|
||
| if (span.Tags is not OpenTelemetryTags tags) | ||
| { | ||
| // while span.Tags can be something other than OpenTelemetryTags the | ||
| // ActivityHandlerCommon.StartActivity requires its "Tags" type to be | ||
| // OpenTelemetryTags to ensure that for all Activity objects that we | ||
| // process will be able to be mapped correctly. | ||
| return; | ||
| } | ||
|
|
||
| if (tags.IsHttpServer()) | ||
| { | ||
| operationName = "http.server.request"; | ||
| } | ||
| else if (tags.IsHttpClient()) | ||
| { | ||
| operationName = "http.client.request"; | ||
| } | ||
| else if (tags.IsDatabase()) | ||
| { | ||
| operationName = $"{tags.DbSystem}.query"; | ||
| } | ||
| else if (tags.IsMessaging()) | ||
| { | ||
| operationName = $"{tags.MessagingSystem}.{tags.MessagingOperation}"; | ||
|
There was a problem hiding this comment. More a question for the RFC rather than this PR, but is it still preferable to use There was a problem hiding this comment. I think the semantics that we are going off of from OpenTelemetry (still says their status is But after I got the There was a problem hiding this comment. A question for @zacharycmontoya perhaps then There was a problem hiding this comment. I think it makes sense for the other operation names to be |
||
| } | ||
| else if (tags.IsAwsClient()) | ||
| { | ||
| operationName = !string.IsNullOrEmpty(tags.RpcService) ? $"aws.{tags.RpcService}.request" : "aws.client.request"; | ||
| } | ||
| else if (tags.IsRpcClient()) | ||
| { | ||
| operationName = $"{tags.RpcSystem}.client.request"; | ||
| } | ||
| else if (tags.IsRpcServer()) | ||
| { | ||
| operationName = $"{tags.RpcSystem}.server.request"; | ||
| } | ||
| else if (tags.IsFaasServer()) | ||
| { | ||
| operationName = $"{tags.FaasTrigger}.invoke"; | ||
| } | ||
| else if (tags.IsFaasClient()) | ||
| { | ||
| operationName = $"{tags.FaasInvokedProvider}.{tags.FaasInvokedName}.invoke"; | ||
| } | ||
| else if (tags.IsGraphQLServer()) | ||
| { | ||
| operationName = "graphql.server.request"; | ||
| } | ||
|
Comment on lines
+71
to
+74
There was a problem hiding this comment. We can't recognize There was a problem hiding this comment. Good question, the semantics for this just have it as When I made the parametric tests I looked at the PR that added the spec (OpenTelemetry) for this and there was some discussion around whether or not it should also be for |
||
| else if (tags.IsGenericServer()) | ||
| { | ||
| operationName = !string.IsNullOrEmpty(tags.NetworkProtocolName) ? $"{tags.NetworkProtocolName}.server.request" : "server.request"; | ||
| } | ||
| else if (tags.IsGenericClient()) | ||
| { | ||
| operationName = !string.IsNullOrEmpty(tags.NetworkProtocolName) ? $"{tags.NetworkProtocolName}.client.request" : "client.request"; | ||
| } | ||
| else | ||
| { | ||
| operationName = !string.IsNullOrEmpty(tags.SpanKind) ? tags.SpanKind : "otel_unknown"; | ||
| } | ||
|
|
||
| span.OperationName = operationName.ToLower(); | ||
bouwkast marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| private static bool IsHttpServer(this OpenTelemetryTags tags) | ||
| { | ||
| return tags.SpanKind == SpanKinds.Server && !string.IsNullOrEmpty(tags.HttpRequestMethod); | ||
| } | ||
|
|
||
| private static bool IsHttpClient(this OpenTelemetryTags tags) | ||
| { | ||
| return tags.SpanKind == SpanKinds.Client && !string.IsNullOrEmpty(tags.HttpRequestMethod); | ||
| } | ||
|
|
||
| private static bool IsDatabase(this OpenTelemetryTags tags) | ||
| { | ||
| return tags.SpanKind == SpanKinds.Client && !string.IsNullOrEmpty(tags.DbSystem); | ||
| } | ||
|
|
||
| private static bool IsMessaging(this OpenTelemetryTags tags) | ||
| { | ||
| return (tags.SpanKind == SpanKinds.Client || | ||
| tags.SpanKind == SpanKinds.Server || | ||
| tags.SpanKind == SpanKinds.Producer || | ||
| tags.SpanKind == SpanKinds.Consumer) | ||
| && !string.IsNullOrEmpty(tags.MessagingSystem) && | ||
| !string.IsNullOrEmpty(tags.MessagingOperation); | ||
| } | ||
|
|
||
| private static bool IsAwsClient(this OpenTelemetryTags tags) | ||
| { | ||
| return tags.SpanKind == SpanKinds.Client && string.Equals(tags.RpcSystem, "aws-api", StringComparison.OrdinalIgnoreCase); | ||
| } | ||
|
|
||
| private static bool IsRpcClient(this OpenTelemetryTags tags) | ||
| { | ||
| return tags.SpanKind == SpanKinds.Client && !string.IsNullOrEmpty(tags.RpcSystem); | ||
| } | ||
|
|
||
| private static bool IsRpcServer(this OpenTelemetryTags tags) | ||
| { | ||
| return tags.SpanKind == SpanKinds.Server && !string.IsNullOrEmpty(tags.RpcSystem); | ||
| } | ||
|
|
||
| private static bool IsFaasServer(this OpenTelemetryTags tags) | ||
| { | ||
| return tags.SpanKind == SpanKinds.Server && !string.IsNullOrEmpty(tags.FaasTrigger); | ||
| } | ||
|
|
||
| private static bool IsFaasClient(this OpenTelemetryTags tags) | ||
| { | ||
| return tags.SpanKind == SpanKinds.Client && !string.IsNullOrEmpty(tags.FaasInvokedProvider) && !string.IsNullOrEmpty(tags.FaasInvokedName); | ||
| } | ||
|
|
||
| private static bool IsGraphQLServer(this OpenTelemetryTags tags) | ||
| { | ||
| return tags.SpanKind == SpanKinds.Server && !string.IsNullOrEmpty(tags.GraphQlOperationType); | ||
| } | ||
|
|
||
| private static bool IsGenericServer(this OpenTelemetryTags tags) | ||
| { | ||
| return tags.SpanKind == SpanKinds.Server; | ||
| } | ||
|
|
||
| private static bool IsGenericClient(this OpenTelemetryTags tags) | ||
| { | ||
| return tags.SpanKind == SpanKinds.Client; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you double check that all callers of
ActivityHandlerCommon.ActivityStartedare updated? I think that the Azure Service Bus handler may need to be updated to carry some of the new tagsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what I opted to do was update
ActivityHandlerCommon.ActivityStartedto takeOpenTelemetryTagsinstead of anITags.Then I made the
AzureServiceBusTagsinheritOpenTelemetryTagsand that seemed to get the mapping working correctly for the operation name.I'm not super sure whether or not that'll cause issues, I did have it complain about a duplicate
span.kindtag, so I removed the[Tag]attribute on theAzureServiceBusTags.SpanKindproperty and just overrideOpenTelemetryTags.SpanKind, which seemed to do the trick.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In terms of using inheritance, that should be absolutely fine, we do it with various tags 🙂
The only downside to the approach I can see is the increased memory footprint of the big fat
OpenTelemetryTagsinstances 😄