-
Notifications
You must be signed in to change notification settings - Fork 530
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
metrics-generator: include messaging systems and databases in service graphs #1576
metrics-generator: include messaging systems and databases in service graphs #1576
Conversation
For testing: you can run this Tempo image in the intro-to-mlt demo https://github.com/grafana/intro-to-mlt/tree/kvrhdn/add-rabbitmq |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't have a lot to comment. It looks good 👍
edge := newEdge(key, s.ttl) | ||
update(edge) | ||
|
||
if edge.isComplete() { | ||
s.onComplete(edge) | ||
return true, nil | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement 👍
I'll update the corresponding docs in a separate PR since I've a couple of changes pending and it's just going to clutter this PR. |
case v1_trace.Span_SPAN_KIND_PRODUCER: | ||
connectionType = store.MessagingSystem | ||
fallthrough |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add a comment here to emphasise why the producer case was placed before client and similarly for consumer <> server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoa, I've never seen fallthrough
before. Neat.
What this PR does:
Include databases and requests across messaging systems in service graphs metrics. We rely on the OpenTelemetry semantic conventions to detect when a span is involved in one of these systems.
Links:
Messaging system: span kind will be
consumer
andproducer
. We addconnection_type="messaging_system"
to the metrics.Database: span kind will be
client
but there will be no matchingserver
span (the database is most likely not instrumented). We copy the values from the client side and use that to populate the server latency. Downside: you can't track the client and server-side latency of a request.We add
connection_type="database"
to the metrics.Which issue(s) this PR fixes:
Fixes #1435
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]