Add OTEL Metrics #629
Add OTEL Metrics #629
Conversation
|
Any updates on this PR, since? I can see it has been outstanding for a few months now. |
not sure since it's still in draft state. @iby-dev did you get a chance to test it at all? maybe with aspire? |
|
Hallo, my TL is against aspire so we won't be using it. There is a container that one can use auto extracts metrics from a running nats server which i may have a look at it. I will come back to this depending on how i get on. It is at the bottom of my last sadly. |
| public const string PendingMessagesInstrumentName = $"{InstrumentPrefix}.pending.messages"; | ||
| public const string SentBytesInstrumentName = $"{InstrumentPrefix}.sent.bytes"; | ||
| public const string ReceivedBytesInstrumentName = $"{InstrumentPrefix}.received.bytes"; | ||
| public const string SentMessagesInstrumentName = $"{InstrumentPrefix}.sent.messages"; |
There was a problem hiding this comment.
Should be messaging.client.sent.messages as per https://opentelemetry.io/docs/specs/semconv/messaging/messaging-metrics/#metric-messagingclientsentmessages
| public const string SentBytesInstrumentName = $"{InstrumentPrefix}.sent.bytes"; | ||
| public const string ReceivedBytesInstrumentName = $"{InstrumentPrefix}.received.bytes"; | ||
| public const string SentMessagesInstrumentName = $"{InstrumentPrefix}.sent.messages"; | ||
| public const string ReceivedMessagesInstrumentName = $"{InstrumentPrefix}.received.messages"; |
There was a problem hiding this comment.
Should be messaging.client.consumed.messages as per https://opentelemetry.io/docs/specs/semconv/messaging/messaging-metrics/#metric-messagingclientconsumedmessages
| public class NatsMetrics | ||
| { | ||
| public const string MeterName = "NATS.Client"; | ||
| public const string PendingMessagesInstrumentName = $"{InstrumentPrefix}.pending.messages"; |
There was a problem hiding this comment.
Request has been raised to add this open-telemetry/semantic-conventions#2263
| public const string SentBytesInstrumentName = $"{InstrumentPrefix}.sent.bytes"; | ||
| public const string ReceivedBytesInstrumentName = $"{InstrumentPrefix}.received.bytes"; |
There was a problem hiding this comment.
Suggest raising a request to add these to registry just like we have in http https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#metric-httpserverrequestbodysize
There was a problem hiding this comment.
Request Raised as open-telemetry/semantic-conventions#2264
| private static readonly Counter<long> _sentMessagesCounter; | ||
| private static readonly Counter<long> _receivedMessagesCounter; | ||
|
|
||
| static NatsMetrics() |
There was a problem hiding this comment.
Attributes for metrics are missing as per https://opentelemetry.io/docs/specs/semconv/messaging/messaging-metrics/#common-metrics
|
|
||
| namespace NATS.Client.Core.Internal; | ||
|
|
||
| public class NatsMetrics |
There was a problem hiding this comment.
Required metrics ie duration is missing as per https://opentelemetry.io/docs/specs/semconv/messaging/messaging-metrics/#common-metrics
|
|
||
| namespace NATS.Client.Core.Internal; | ||
|
|
||
| public class NatsMetrics |
There was a problem hiding this comment.
A request has been raised to register the system name: open-telemetry/semantic-conventions#2266
|
thanks for the comments @thompson-tomo. we might have to open a new pr not sure if @divyeshio is still interested in this feature. been a while. |
|
Hey @mtmk @thompson-tomo, sorry I've been inactive, I won't be able to work on this. |
thanks for the update @divyeshio 💯 thank you for kicking this off and your hard work ❤️ |
|
Thank you @thompson-tomo |
Fixes #316
NatsStasfunctionality with System.Diagnostics.Metrics implementation