Revamp messaging metrics to support generic operations #1006

lmolkova · 2024-05-04T04:44:53Z

Fixes #947, #937

Changes

deprecate messaging.operation.type
make messaging.operation.name required.
update span name accordingly (and following DB semconv pattern of {operation} {destination})
update metrics:
- unify messaging.publish.duration and messaging.receive.duration in one messaging.client.operation.duration
- rename messaging.publish.messages to messaging.client.produced.messages
- unify messaging.receive.messages and messaging.process.messages in one messaging.client.consumed.messages
- rename messaging.process.duration to messaging.client.process.duration

Merge requirement checklist

CONTRIBUTING.md guidelines followed.
Change log entry added, according to the guidelines in When to add a changelog entry.
- If your PR does not need a change log, start the PR title with [chore]
schema-next.yaml updated with changes to existing conventions.

docs/messaging/messaging-metrics.md

pyohannes

Thanks @lmolkova, I think this is going in a good direction.

docs/attributes-registry/messaging.md

docs/messaging/messaging-metrics.md

.chloggen/1006.yaml

model/trace/messaging.yaml

docs/messaging/messaging-spans.md

docs/messaging/messaging-metrics.md

docs/messaging/messaging-spans.md

schema-next.yaml

docs/messaging/azure-messaging.md

pyohannes

Thanks a lot @lmolkova, it feels we're getting quite close!

docs/messaging/messaging-metrics.md

model/metrics/messaging-metrics.yaml

docs/messaging/messaging-spans.md

pyohannes

I have some smaller remarks, but nothing that should block merging this. I'm looking forward to first prototypes based on these changes.

docs/messaging/messaging-spans.md

.chloggen/1006.yaml

trask · 2024-06-07T21:10:41Z

deprecate messaging.operation.type

and

unify ... in one messaging.client.operation.duration

looks like drift on this parts of PR description

trask · 2024-06-07T21:28:42Z

model/metrics/messaging-metrics.yaml

+  # durations
+  - id: metric.messaging.client.request.duration
+    type: metric
+    metric_name: messaging.client.request.duration


naming it "request" seems like a good choice over "operation" in order to help mitigate the confusion around whether this also includes "process" times (which have "messaging.operation.name" and so seem like they should be included in anything called "operation")

trask · 2024-06-07T21:36:02Z

model/metrics/messaging-metrics.yaml

+      The metric MUST be reported once per message delivery. For example, if receiving and processing operations are both instrumented for a single message delivery, the number of received messages
+      is reported when then message is received and not reported for the processing operation.


maybe MUST -> SHOULD?

this could be tricky to implement / coordinate, e.g. when we instrument Spring messaging to capture "process" operations, and Spring uses Kafka polling under the hood which is also instrumented to capture "receive" operation

trask · 2024-06-07T21:36:42Z

model/metrics/messaging-metrics.yaml

+    metric_name: messaging.client.published.messages
+    brief: "Number of messages producer attempted to publish to the broker."
+    note: >
+      This metric MUST NOT count messages that were created, but aren't being published yet.


Suggested change

This metric MUST NOT count messages that were created, but aren't being published yet.

This metric MUST NOT count messages that were created but haven't been published yet.

trask · 2024-06-07T21:37:15Z

model/metrics/messaging-metrics.yaml

+    metric_name: messaging.client.request.duration
+    brief: "Duration of messaging operation initiated by a producer or consumer."
+    note: >
+      This metric SHOULD NOT be used to report processing duration - processing duration is reported in `messaging.process.duration` metric.


Suggested change

This metric SHOULD NOT be used to report processing duration - processing duration is reported in `messaging.process.duration` metric.

This metric SHOULD NOT be used to report processing duration - processing duration is reported in `messaging.client.process.duration` metric.

lmolkova force-pushed the messaging-revamp-metrics branch 2 times, most recently from 4511bc0 to 86ee3c9 Compare May 4, 2024 05:14

lmolkova commented May 4, 2024

View reviewed changes

docs/messaging/messaging-metrics.md Outdated Show resolved Hide resolved

lmolkova commented May 4, 2024

View reviewed changes

docs/messaging/messaging-metrics.md Outdated Show resolved Hide resolved

lmolkova force-pushed the messaging-revamp-metrics branch from 86ee3c9 to 0593db2 Compare May 7, 2024 23:38

lmolkova marked this pull request as ready for review May 8, 2024 05:01

lmolkova requested review from a team as code owners May 8, 2024 05:01

github-actions bot assigned joaopgrassi May 8, 2024

pyohannes reviewed May 8, 2024

View reviewed changes

docs/attributes-registry/messaging.md Outdated Show resolved Hide resolved

docs/messaging/messaging-metrics.md Outdated Show resolved Hide resolved

docs/messaging/messaging-metrics.md Outdated Show resolved Hide resolved

docs/messaging/messaging-metrics.md Outdated Show resolved Hide resolved

lmolkova force-pushed the messaging-revamp-metrics branch from 18f8142 to 19cd105 Compare May 9, 2024 04:34

trask reviewed May 9, 2024

View reviewed changes

.chloggen/1006.yaml Outdated Show resolved Hide resolved

trask reviewed May 9, 2024

View reviewed changes

.chloggen/1006.yaml Outdated Show resolved Hide resolved

trask reviewed May 9, 2024

View reviewed changes

model/trace/messaging.yaml Outdated Show resolved Hide resolved

model/trace/messaging.yaml Outdated Show resolved Hide resolved

docs/messaging/messaging-spans.md Outdated Show resolved Hide resolved

pyohannes reviewed May 23, 2024

View reviewed changes

docs/messaging/messaging-metrics.md Outdated Show resolved Hide resolved

docs/messaging/messaging-spans.md Outdated Show resolved Hide resolved

docs/messaging/messaging-spans.md Outdated Show resolved Hide resolved

lmolkova commented May 24, 2024

View reviewed changes

schema-next.yaml Outdated Show resolved Hide resolved

lmolkova force-pushed the messaging-revamp-metrics branch 2 times, most recently from 4010ef2 to 203ebb3 Compare May 30, 2024 01:57

lmolkova commented May 30, 2024

View reviewed changes

docs/messaging/azure-messaging.md Outdated Show resolved Hide resolved

lmolkova commented May 30, 2024

View reviewed changes

docs/messaging/azure-messaging.md Outdated Show resolved Hide resolved

lmolkova force-pushed the messaging-revamp-metrics branch 5 times, most recently from 1b78dd5 to 69bfd72 Compare May 30, 2024 22:07

pyohannes reviewed May 31, 2024

View reviewed changes

docs/messaging/messaging-metrics.md Show resolved Hide resolved

docs/messaging/messaging-metrics.md Outdated Show resolved Hide resolved

model/metrics/messaging-metrics.yaml Show resolved Hide resolved

docs/messaging/messaging-spans.md Outdated Show resolved Hide resolved

lmolkova mentioned this pull request Jun 2, 2024

Don't render deprecated enum members in semconv tables #1110

Merged

3 tasks

pyohannes mentioned this pull request Jun 3, 2024

Be more specific about span kinds for messaging operations #1112

Open

lmolkova force-pushed the messaging-revamp-metrics branch from eae8b8c to de27752 Compare June 4, 2024 00:33

lmolkova commented Jun 4, 2024

View reviewed changes

docs/messaging/messaging-spans.md Show resolved Hide resolved

pyohannes approved these changes Jun 4, 2024

View reviewed changes

docs/messaging/messaging-spans.md Outdated Show resolved Hide resolved

docs/messaging/messaging-spans.md Outdated Show resolved Hide resolved

docs/messaging/messaging-spans.md Show resolved Hide resolved

docs/messaging/messaging-spans.md Show resolved Hide resolved

lmolkova force-pushed the messaging-revamp-metrics branch from de27752 to 861abd2 Compare June 5, 2024 19:52

joaopgrassi approved these changes Jun 7, 2024

View reviewed changes

.chloggen/1006.yaml Show resolved Hide resolved

lmolkova added 10 commits June 7, 2024 12:54

Revamp messaging metrics to support generic operations

434bcbd

oops

637e2ca

update based on the sig discussion

eee0d6e

return operation type to examples

97ab186

update examples

ad6de18

nits

55ae08b

up

056454e

undo span kinds and remove deliver

f534dc2

span name code review

cf5295e

lint

9504ad5

lmolkova force-pushed the messaging-revamp-metrics branch from b8480d3 to 9504ad5 Compare June 7, 2024 19:55

trask reviewed Jun 7, 2024

View reviewed changes

Merge branch 'main' into messaging-revamp-metrics

cbe6c53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revamp messaging metrics to support generic operations #1006

Revamp messaging metrics to support generic operations #1006

lmolkova commented May 4, 2024 •

edited

pyohannes left a comment

pyohannes left a comment

pyohannes left a comment

trask commented Jun 7, 2024 •

edited

trask Jun 7, 2024

trask Jun 7, 2024

trask Jun 7, 2024

trask Jun 7, 2024

		The metric MUST be reported once per message delivery. For example, if receiving and processing operations are both instrumented for a single message delivery, the number of received messages
		is reported when then message is received and not reported for the processing operation.

	This metric MUST NOT count messages that were created, but aren't being published yet.
	This metric MUST NOT count messages that were created but haven't been published yet.

	This metric SHOULD NOT be used to report processing duration - processing duration is reported in `messaging.process.duration` metric.
	This metric SHOULD NOT be used to report processing duration - processing duration is reported in `messaging.client.process.duration` metric.

Revamp messaging metrics to support generic operations #1006

Are you sure you want to change the base?

Revamp messaging metrics to support generic operations #1006

Conversation

lmolkova commented May 4, 2024 • edited

Changes

Merge requirement checklist

pyohannes left a comment

Choose a reason for hiding this comment

pyohannes left a comment

Choose a reason for hiding this comment

pyohannes left a comment

Choose a reason for hiding this comment

trask commented Jun 7, 2024 • edited

trask Jun 7, 2024

Choose a reason for hiding this comment

trask Jun 7, 2024

Choose a reason for hiding this comment

trask Jun 7, 2024

Choose a reason for hiding this comment

trask Jun 7, 2024

Choose a reason for hiding this comment

lmolkova commented May 4, 2024 •

edited

trask commented Jun 7, 2024 •

edited