-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[AzureMonitorExporter] Add Microsoft override attributes for Application Insights compatibility #54023
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
base: main
Are you sure you want to change the base?
[AzureMonitorExporter] Add Microsoft override attributes for Application Insights compatibility #54023
Changes from all commits
7cd03f8
d27973f
295d6e0
5b26347
fd8924b
c984023
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 |
|---|---|---|
|
|
@@ -63,7 +63,7 @@ public static void Clear(ref AzMonList list) | |
|
|
||
| for (int i = 0; i < length; i++) | ||
| { | ||
| if (ReferenceEquals(list[i].Key, tagName)) | ||
| if (string.Equals(list[i].Key, tagName, StringComparison.Ordinal)) | ||
|
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. Not a big deal, just caught my eye. How did this even work in the previous version? Some basic tests should have caught it, no?
Contributor
Author
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. Our tests only highlighted this issue. All these days we had semantics that are in OpenTelemetry, which we could use as reference. But now we have to move to support both internal and OpenTelemetry semantics. Hence the change. 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. How did this even work before? Looks like it would have returned false always, no? |
||
| { | ||
| return list[i].Value; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -175,5 +175,69 @@ internal static class SemanticConventions | |
| public const string AttributeDbQuerySummary = "db.query.summary"; | ||
| public const string AttributeDbQueryText = "db.query.text"; | ||
| public const string AttributeDbStoredProcedureName = "db.stored_procedure.name"; | ||
|
|
||
| // Microsoft Application Insights Override Attributes | ||
| // These attributes allow explicit mapping from Application Insights fields to OpenTelemetry attributes | ||
| // When present, these override computed values from standard semantic conventions | ||
|
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. NIT: should it say when present, these ..... from OTel event ? |
||
|
|
||
| /// <summary> | ||
| /// Override attribute for dependency data field. | ||
| /// When present, takes precedence over computed data from semantic conventions. | ||
|
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. NIT: ...data from OTel event might be appropriate, instead of semantic convention
Contributor
Author
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. Event has a specific meaning in OTel (it refers to the Events data model), so I used "semantic conventions" to avoid confusion. That said, I don't have a strong preference - if you think 'data from OTel event' reads better will change it. |
||
| /// </summary> | ||
| public const string AttributeMicrosoftDependencyData = "microsoft.dependency.data"; | ||
|
|
||
| /// <summary> | ||
| /// Override attribute for dependency name field. | ||
| /// When present, takes precedence over computed name from semantic conventions. | ||
| /// </summary> | ||
| public const string AttributeMicrosoftDependencyName = "microsoft.dependency.name"; | ||
|
|
||
| /// <summary> | ||
| /// Override attribute for operation name field. | ||
| /// When present, takes precedence over computed operation name. | ||
| /// </summary> | ||
| public const string AttributeMicrosoftOperationName = "microsoft.operation_name"; | ||
|
|
||
| /// <summary> | ||
| /// Override attribute for dependency result code field. | ||
| /// When present, takes precedence over computed result code from semantic conventions. | ||
| /// </summary> | ||
| public const string AttributeMicrosoftDependencyResultCode = "microsoft.dependency.resultCode"; | ||
|
|
||
| /// <summary> | ||
| /// Override attribute for dependency target field. | ||
| /// When present, takes precedence over computed target from semantic conventions. | ||
| /// </summary> | ||
| public const string AttributeMicrosoftDependencyTarget = "microsoft.dependency.target"; | ||
|
|
||
| /// <summary> | ||
| /// Override attribute for dependency type field. | ||
| /// When present, takes precedence over computed type from semantic conventions. | ||
| /// </summary> | ||
| public const string AttributeMicrosoftDependencyType = "microsoft.dependency.type"; | ||
|
|
||
| /// <summary> | ||
| /// Override attribute for request name field. | ||
| /// When present, takes precedence over Activity.DisplayName. | ||
| /// </summary> | ||
| public const string AttributeMicrosoftRequestName = "microsoft.request.name"; | ||
|
|
||
| /// <summary> | ||
| /// Override attribute for request URL field. | ||
| /// When present, takes precedence over computed URL from semantic conventions. | ||
| /// </summary> | ||
| public const string AttributeMicrosoftRequestUrl = "microsoft.request.url"; | ||
|
|
||
| /// <summary> | ||
| /// Override attribute for request source field. | ||
| /// When present, takes precedence over computed source from semantic conventions. | ||
| /// </summary> | ||
| public const string AttributeMicrosoftRequestSource = "microsoft.request.source"; | ||
|
|
||
| /// <summary> | ||
| /// Override attribute for request result code field. | ||
| /// When present, takes precedence over computed result code from semantic conventions. | ||
| /// </summary> | ||
| public const string AttributeMicrosoftRequestResultCode = "microsoft.request.resultCode"; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,11 @@ internal static (List<TelemetryItem> TelemetryItems, TelemetrySchemaTypeCounter | |
| { | ||
| case TelemetryType.Request: | ||
| var requestData = new RequestData(Version, activity, ref activityTagsProcessor); | ||
| requestData.Name = telemetryItem.Tags.TryGetValue(ContextTagKeys.AiOperationName.ToString(), out var operationName) ? operationName.Truncate(SchemaConstants.RequestData_Name_MaxLength) : activity.DisplayName.Truncate(SchemaConstants.RequestData_Name_MaxLength); | ||
| // Only set Name if not already set by override attribute | ||
| if (string.IsNullOrEmpty(requestData.Name)) | ||
| { | ||
| requestData.Name = telemetryItem.Tags.TryGetValue(ContextTagKeys.AiOperationName.ToString(), out var operationName) ? operationName.Truncate(SchemaConstants.RequestData_Name_MaxLength) : activity.DisplayName.Truncate(SchemaConstants.RequestData_Name_MaxLength); | ||
|
Member
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. nit: break down into multiple lines |
||
| } | ||
| telemetryItem.Data = new MonitorBase | ||
| { | ||
| BaseType = "RequestData", | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.