Azure Monitor Exporter - Parse dependency types #15533
Conversation
rajkumar-rangaraj
commented
Sep 29, 2020
•
rajkumar-rangaraj
commented
- Added support for azure namespace and component tags
- Added Event Hub extraction support
rajkumar-rangaraj
requested review from
cijothomas,
reyang and
TimothyMothra
as code owners
...r/tests/OpenTelemetry.Exporter.AzureMonitor.UnitTest/HttpParsers/AzureBlobHttpParserTests.cs
Outdated
Show resolved
Hide resolved
sdk/monitor/OpenTelemetry.Exporter.AzureMonitor/src/AzureMonitorTransmitter.cs
Outdated
Show resolved
Hide resolved
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| internal static string GetHost(Dictionary<string, string> tags) | ||
| { | ||
| if (tags != null && tags.TryGetValue(SemanticConventions.AttributeHttpHost, out var host)) |
There was a problem hiding this comment.
Check this out open-telemetry/opentelemetry-dotnet#1236.
There was a problem hiding this comment.
Will get this as a part of next PR. This changes makes this PR complex.
| var testCases = new List<Tuple<string, string, string, Dictionary<string, string>, string>>() | ||
| { | ||
| // Database operations | ||
| Tuple.Create("Create database", "POST", "https://myaccount.documents.azure.com/dbs", defaultProperties, defaultResultCode), |
There was a problem hiding this comment.
@pakrym wish to get your perspective here.
I think we want the DocumentDB SDK to put information indicating the logical operation name, instead of coupling the logic inside the Azure Monitor Exporter.
There was a problem hiding this comment.
That's what we have been doing for track 2 SDKs but I'm not sure how much leeway we would have for existing libraries. We might need to keep these implementations for back-compat.
There was a problem hiding this comment.
Just to throw some thoughts here - would it be possible for us to only support track 2 SDKs in this exporter.
Folks who are using legacy SDKs can use ApplicationInsights SDK, and we'll continue to support it until X% of customers move to track 2 SDKs + OpenTelemetry .NET + Azure Monitor Exporter?
There was a problem hiding this comment.
Agree we shouldn't do any complex domain specific parsing in the exporter. Its good idea to callout that only track2 SDKs are supported in Otel + this exporter.
There was a problem hiding this comment.
Agree we shouldn't do any complex domain specific parsing in the exporter. Its good idea to callout that only track2 SDKs are supported in Otel + this exporter.
Yeah, I think this exporter will only have knowledge about the Azure Monitor schema/endpoint/protocol. It should have no idea what Service Bus, Cosmos DB and Azure Search are.
There was a problem hiding this comment.
Removed all HTTP parsers. Now we check based on az.namespace tag
sdk/monitor/OpenTelemetry.Exporter.AzureMonitor/src/ComponentHelper.cs
Outdated
Show resolved
Hide resolved
sdk/monitor/OpenTelemetry.Exporter.AzureMonitor/src/ComponentHelper.cs
Outdated
Show resolved
Hide resolved
sdk/monitor/OpenTelemetry.Exporter.AzureMonitor/src/HttpHelper.cs
Outdated
Show resolved
Hide resolved
sdk/monitor/OpenTelemetry.Exporter.AzureMonitor/src/HttpParsers/HttpParsingHelper.cs
Outdated
Show resolved
Hide resolved
sdk/monitor/OpenTelemetry.Exporter.AzureMonitor/src/ComponentHelper.cs
Outdated
Show resolved
Hide resolved
sdk/monitor/OpenTelemetry.Exporter.AzureMonitor/src/AzureMonitorConverter.cs
Outdated
Show resolved
Hide resolved
|
These changes should not be part of exporter, it should come from .NET Azure Collector. |
