Skip to content

Avoid using default type mapper#375

Merged
alexeyzimarev merged 2 commits intodevfrom
remove-message-type-from-producedmessage
Sep 9, 2024
Merged

Avoid using default type mapper#375
alexeyzimarev merged 2 commits intodevfrom
remove-message-type-from-producedmessage

Conversation

@alexeyzimarev
Copy link
Copy Markdown
Contributor

@alexeyzimarev alexeyzimarev commented Sep 8, 2024

ProducedMessage struct contains the MessageType property that is used for setting the message type tag in the trace, as well as for additional logging (verbose level). Both those usages required the default type mapper. However, the actual message type is provided by the serialiser, and that value wasn't used in tracing. So, it might occur that when using a custom serialiser that doesn't use the type mapper, a producer would fail because the type isn't registered in the type map although it should work.

This PR adds a SetActivityMessageType protected method to the BaseProducer class. Producer implementations could (not required) use it to set the message type on the trace.

As a side effect it became obvious that setting the message type on the produce activity doesn't work properly when messages are produced in batches. It happens even with implementations that produce messages individually. Therefore, it seems that, although it's still useful, a producer that starts one span for the whole batch isn't the best implementation.

It might be relevant to set the message type tag only if one message is produced.

Producing batches with appropriate traces could potentially be solved by span links. However, there are not many examples of batched produce instrumented with linked spans. The only example I found does the opposite - links one span downstream to multiple spans upstream (batch consume).

In any case, span linking isn't widely supported at the moment, so it might be worth revisiting this later.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Sep 8, 2024

Deploying eventuous-main with  Cloudflare Pages  Cloudflare Pages

Latest commit: 95c55b7
Status: ✅  Deploy successful!
Preview URL: https://6b3ddcf7.eventuous-main.pages.dev
Branch Preview URL: https://remove-message-type-from-pro.eventuous-main.pages.dev

View logs

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 8, 2024

Test Results

 38 files  + 19   38 suites  +19   8m 51s ⏱️ + 4m 48s
189 tests +  2  189 ✅ +  2  0 💤 ±0  0 ❌ ±0 
374 runs  +187  374 ✅ +187  0 💤 ±0  0 ❌ ±0 

Results for commit 95c55b7. ± Comparison against base commit e5a8461.

♻️ This comment has been updated with latest results.

@alexeyzimarev alexeyzimarev merged commit 09bb447 into dev Sep 9, 2024
@alexeyzimarev alexeyzimarev deleted the remove-message-type-from-producedmessage branch September 9, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant