mochi-mqtt / server

The fully compliant, embeddable high-performance Go MQTT v5 server for IoT, smarthome, and pubsub
MIT License
1.29k stars 222 forks source link

[WIP] OTEL Observability #297

Closed IshanDaga closed 6 months ago

IshanDaga commented 1 year ago

Feature

Using OTEL lib to instrument the server. Happy to take suggestions on things that we should track, metrics to add, and how to allow maximum configurability for the Tracer.

Tasks

Pending decisions

IshanDaga commented 1 year ago

@dgduncan @mochi-co Another approach to this could be to have a StartTrace hook that is called after receivePacket We could also add in a trace-modifier hook of sorts, that allows someone to insert the TraceID into the header for MQTTv5 or modify the packet to include TraceID bytes with a separator like in #227

Looking for initial comments and thoughts :)

IshanDaga commented 1 year ago

@mochi-co hey I'm having trouble with the test cases involving sample packets. Most are failing on the require.Equal since now a packet from the server has a context, but the sample packet doesn't.

There's quite a few of these cases, any suggestions on how I could move past this without breaking the current testing method ?

mochi-co commented 1 year ago

@IshanDaga SImplest solution would be to only populate context if OTEL tracing is enabled:

Index: clients.go
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/clients.go b/clients.go
--- a/clients.go    (revision 9d6c78915853d616b25757776b6945885b48df22)
+++ b/clients.go    (date 1697972510469)
@@ -454,7 +454,9 @@
 func (cl *Client) ReadPacket(fh *packets.FixedHeader) (pk packets.Packet, err error) {
    atomic.AddInt64(&cl.ops.info.PacketsReceived, 1)

-   pk.Ctx, pk.Cancel = context.WithCancel(cl.State.open)
+   if cl.ops.options.OTELTracing {
+       pk.Ctx, pk.Cancel = context.WithCancel(cl.State.open)
+   }

    pk.ProtocolVersion = cl.Properties.ProtocolVersion // inherit client protocol version for decoding
    pk.FixedHeader = *fh
IshanDaga commented 1 year ago

@mochi-co makes sense, thanks !

mochi-co commented 6 months ago

@IshanDaga This PR is a bit out of date, I recommend closing it and opening a new one when there's more changes 👍🏻

IshanDaga commented 6 months ago

@mochi-co makes sense, closing for now, will get back on this as soon as i have a little more free time