feat(telemetry): add telemetry support for user agent parsing#90
feat(telemetry): add telemetry support for user agent parsing#90BenjaminAbt wants to merge 14 commits intomainfrom
Conversation
gfoidl
left a comment
There was a problem hiding this comment.
Instead of the event source, I'd prefer to use OTel metrics via the meters.
Especially as we don't write to the event stream (which could be read e.g. via PerfView), and I don't think we need the diagnostic events.
| internal void UserAgentPresent() | ||
| { | ||
| if (!IsEnabled()) return; | ||
| _userAgentPresent?.Increment(); |
There was a problem hiding this comment.
As we don't write an event, should we use metrics (via meter factory) instead?
I'd prefer the OTel metrics here.
There was a problem hiding this comment.
There are too many projects that do not use OTEL - even I/we currently still use native app insights in almost all projects because OTEL features are missing.
There was a problem hiding this comment.
Hm, with UseAzureMonitor I see metrics in Azure. What features are there missing?
There was a problem hiding this comment.
But we would still enforce everybody to use OTEL as foundation... I am not happy with that.
|
@gfoidl , I added a telemetry hub and exposed otel native meters and dotnet metrics. |
gfoidl
left a comment
There was a problem hiding this comment.
For the OTel attributes we should follow the semantic convention (now, because changing it later would be a breaking change).
Sorry for the delay.
src/HttpUserAgentParser.AspNetCore/Telemetry/HttpUserAgentParserAspNetCoreMeters.cs
Outdated
Show resolved
Hide resolved
src/HttpUserAgentParser.AspNetCore/Telemetry/HttpUserAgentParserAspNetCoreMeters.cs
Outdated
Show resolved
Hide resolved
|
@gfoidl no problem. I expected the delay, since it was the holidays. Happy New Year. |
|
@gfoidl there is this note
does this really make sense? We have |
Makes sense, but the value has to be given as floating point number. Then the tools to display should show it as |
src/HttpUserAgentParser.MemoryCache/Telemetry/HttpUserAgentParserMemoryCacheMeters.cs
Show resolved
Hide resolved
src/HttpUserAgentParser.MemoryCache/Telemetry/HttpUserAgentParserMemoryCacheMeters.cs
Show resolved
Hide resolved
src/HttpUserAgentParser.MemoryCache/Telemetry/HttpUserAgentParserMemoryCacheMeters.cs
Show resolved
Hide resolved
src/HttpUserAgentParser/Telemetry/HttpUserAgentParserEventSource.cs
Outdated
Show resolved
Hide resolved
|
the event source is the meter is so calls vs call .. Should that match as best practise? |
Uh oh!
There was an error while loading. Please reload this page.