-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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.
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
[AzureMonitorExporter] Add support new Messaging semantics - Request/Dependency Telemetry #37508
Changes from 9 commits
31bc603
ae1a1af
3a16276
c116632
1ba8100
5905796
105f21c
4a3c387
54c2ebe
30d6ca0
3b173bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,20 +36,29 @@ internal static List<TelemetryItem> OtelToAzureMonitorTrace(Batch<Activity> batc | |
var activityTagsProcessor = EnumerateActivityTags(activity); | ||
telemetryItem = new TelemetryItem(activity, ref activityTagsProcessor, azureMonitorResource, instrumentationKey); | ||
|
||
// Check for Exceptions events | ||
if (activity.Events.Any()) | ||
{ | ||
AddTelemetryFromActivityEvents(activity, telemetryItem, telemetryItems); | ||
} | ||
|
||
switch (activity.GetTelemetryType()) | ||
{ | ||
case TelemetryType.Request: | ||
telemetryItem.Data = new MonitorBase | ||
if (activity.Kind == ActivityKind.Server) | ||
{ | ||
BaseType = "RequestData", | ||
BaseData = new RequestData(Version, activity, ref activityTagsProcessor) | ||
}; | ||
var (requestUrl, operationName) = GetHttpOperationNameAndUrl(activity.DisplayName, activityTagsProcessor.activityType, ref activityTagsProcessor.MappedTags); | ||
telemetryItem.Tags[ContextTagKeys.AiOperationName.ToString()] = operationName; | ||
telemetryItem.Tags[ContextTagKeys.AiLocationIp.ToString()] = TraceHelper.GetLocationIp(ref activityTagsProcessor.MappedTags); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @vishweshbankwar I think for new HTTP semantics, we need to add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also UserAgent There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TimothyMothra Is there a plan to add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no "plan" at the moment. The spec describes |
||
|
||
telemetryItem.Data = new MonitorBase | ||
{ | ||
BaseType = "RequestData", | ||
BaseData = new RequestData(Version, operationName, requestUrl, activity, ref activityTagsProcessor) | ||
}; | ||
} | ||
else | ||
{ | ||
telemetryItem.Data = new MonitorBase | ||
{ | ||
BaseType = "RequestData", | ||
BaseData = new RequestData(Version, activity, ref activityTagsProcessor) | ||
}; | ||
} | ||
break; | ||
case TelemetryType.Dependency: | ||
telemetryItem.Data = new MonitorBase | ||
|
@@ -60,6 +69,12 @@ internal static List<TelemetryItem> OtelToAzureMonitorTrace(Batch<Activity> batc | |
break; | ||
} | ||
|
||
// Check for Exceptions events | ||
if (activity.Events.Any()) | ||
{ | ||
AddTelemetryFromActivityEvents(activity, telemetryItem, telemetryItems); | ||
} | ||
|
||
activityTagsProcessor.Return(); | ||
telemetryItems.Add(telemetryItem); | ||
} | ||
|
@@ -171,7 +186,7 @@ internal static string GetOperationName(Activity activity, ref AzMonList MappedT | |
return activity.DisplayName; | ||
} | ||
|
||
internal static string GetNewSchemaOperationName(Activity activity, string? url, ref AzMonList MappedTags) | ||
internal static string GetNewSchemaOperationName(Activity activity, string? url, ref AzMonList MappedTags) | ||
{ | ||
var httpMethod = AzMonList.GetTagValue(ref MappedTags, SemanticConventions.AttributeHttpRequestMethod)?.ToString(); | ||
if (!string.IsNullOrWhiteSpace(httpMethod)) | ||
|
@@ -195,6 +210,42 @@ internal static string GetNewSchemaOperationName(Activity activity, string? url, | |
return activity.DisplayName; | ||
} | ||
|
||
internal static (string? RequestUrl, string? OperationName) GetHttpOperationNameAndUrl(string activityDisplayName, OperationType operationType, ref AzMonList httpMappedTags) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this logic also needs to change. We no longer need to rely on url in new schema as we will have the path available. Simple Can be done as a follow up PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, thought about it and felt better to be followed in a new PR. |
||
{ | ||
string? httpMethod; | ||
string? httpUrl; | ||
|
||
if (operationType.HasFlag(OperationType.V2)) | ||
{ | ||
httpUrl = httpMappedTags.GetNewSchemaRequestUrl(); | ||
httpMethod = AzMonList.GetTagValue(ref httpMappedTags, SemanticConventions.AttributeHttpRequestMethod)?.ToString(); | ||
} | ||
else | ||
{ | ||
httpUrl = AzMonList.GetTagValue(ref httpMappedTags, SemanticConventions.AttributeHttpUrl)?.ToString(); | ||
httpMethod = AzMonList.GetTagValue(ref httpMappedTags, SemanticConventions.AttributeHttpMethod)?.ToString(); | ||
} | ||
|
||
if (!string.IsNullOrWhiteSpace(httpMethod)) | ||
{ | ||
var httpRoute = AzMonList.GetTagValue(ref httpMappedTags, SemanticConventions.AttributeHttpRoute)?.ToString(); | ||
|
||
// ASP.NET instrumentation assigns route as {controller}/{action}/{id} which would result in the same name for different operations. | ||
// To work around that we will use path from httpUrl. | ||
if (httpRoute?.Contains("{controller}") == false) | ||
{ | ||
return (RequestUrl: httpUrl, OperationName: $"{httpMethod} {httpRoute}"); | ||
} | ||
|
||
if (!string.IsNullOrWhiteSpace(httpUrl) && Uri.TryCreate(httpUrl!.ToString(), UriKind.RelativeOrAbsolute, out var uri) && uri.IsAbsoluteUri) | ||
{ | ||
return (RequestUrl: httpUrl, OperationName: $"{httpMethod} {uri.AbsolutePath}"); | ||
} | ||
} | ||
|
||
return (RequestUrl: httpUrl, OperationName: activityDisplayName); | ||
} | ||
|
||
private static void AddTelemetryFromActivityEvents(Activity activity, TelemetryItem telemetryItem, List<TelemetryItem> telemetryItems) | ||
{ | ||
foreach (ref readonly var @event in activity.EnumerateEvents()) | ||
|
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.
@TimothyMothra We may need a follow up PR here to check if this requires Truncation. Truncation was not a part of existing implementation.
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.
Tags
don't have a max length defined in the swagger.https://github.com/Azure/azure-rest-api-specs/blob/55fc71748c6f51a049fb77aa162ca797aabfbae6/specification/applicationinsights/data-plane/Monitor.Exporters/preview/v2.1/swagger.json#L872C18-L872C18
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.
I found the length for AIOperationName. It's in an internal repo. I'll get this updated