Skip to content

feat: Add message parameter to otel_debug macro#1973

Merged
jmacd merged 13 commits intoopen-telemetry:mainfrom
andborja:andborja/MsgToDebugMacro
Feb 6, 2026
Merged

feat: Add message parameter to otel_debug macro#1973
jmacd merged 13 commits intoopen-telemetry:mainfrom
andborja:andborja/MsgToDebugMacro

Conversation

@andborja
Copy link
Copy Markdown
Contributor

@andborja andborja commented Feb 5, 2026

Change Summary

Adding "message" attribute to the otel_debug macro.

Part of #1972

What issue does this PR close?

  • Closes NA

How are these changes tested?

Building the package

Are there any user-facing changes?

No

@andborja andborja requested a review from a team as a code owner February 5, 2026 20:44
@github-actions github-actions Bot added the rust Pull requests that update Rust code label Feb 5, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 5, 2026

Codecov Report

❌ Patch coverage is 86.48649% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.71%. Comparing base (e95eee9) to head (fbe852c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1973      +/-   ##
==========================================
- Coverage   85.72%   85.71%   -0.01%     
==========================================
  Files         513      513              
  Lines      162056   162070      +14     
==========================================
+ Hits       138919   138926       +7     
- Misses      22603    22610       +7     
  Partials      534      534              
Components Coverage Δ
otap-dataflow 87.52% <86.48%> (-0.01%) ⬇️
query_abstraction 80.61% <ø> (ø)
query_engine 90.23% <ø> (ø)
syslog_cef_receivers ∅ <ø> (∅)
otel-arrow-go 53.50% <ø> (ø)
quiver 91.61% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread rust/otap-dataflow/crates/telemetry/src/internal_events.rs Outdated
@jmacd
Copy link
Copy Markdown
Contributor

jmacd commented Feb 5, 2026

Part of #1972

Copy link
Copy Markdown
Contributor

@jmacd jmacd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1972

@andborja what do you think of un-quoting the first argument, so RECEIVER_CREATE_START instead of "receiver.create.start" for example?

Comment thread rust/otap-dataflow/crates/engine/src/lib.rs Outdated
Comment thread rust/otap-dataflow/crates/telemetry/src/tracing_init.rs
Copy link
Copy Markdown
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not opposed to the idea of using Human readable message in logs. Requesting changes to address the following.

  1. Make it optional. Only few scenarios benefit from human readable message, so no need of making it mandatory.
  2. Make it for all severity, not just debug.
  3. Don't add message for the sake of adding it. If the event.name says "component.started", what additional value does a message "Component is started" providing?

open-telemetry/semantic-conventions#3343 We are totally aligned with this proposal in OTel conventions as well with the above. (this PR in semantic convention is, in turn, aligned with OTel specification too)

@andborja
Copy link
Copy Markdown
Contributor Author

andborja commented Feb 5, 2026

I am not opposed to the idea of using Human readable message in logs. Requesting changes to address the following.

  1. Make it optional. Only few scenarios benefit from human readable message, so no need of making it mandatory.
  2. Make it for all severity, not just debug.
  3. Don't add message for the sake of adding it. If the event.name says "component.started", what additional value does a message "Component is started" providing?

open-telemetry/semantic-conventions#3343 We are totally aligned with this proposal in OTel conventions as well with the above. (this PR in semantic convention is, in turn, aligned with OTel specification too)

  1. Agree.
  2. This PR is for debug only. I'll send others for the rest.
  3. Agree.

Comment thread rust/otap-dataflow/crates/telemetry/src/internal_events.rs Outdated
@andborja andborja requested a review from cijothomas February 6, 2026 00:51
@jmacd jmacd dismissed cijothomas’s stale review February 6, 2026 18:29

Cijo, Andres, and I discussed.
We're going after requirements, not much agreement on style/syntax.

@jmacd jmacd added this pull request to the merge queue Feb 6, 2026
@jmacd jmacd removed this pull request from the merge queue due to a manual request Feb 6, 2026
@cijothomas
Copy link
Copy Markdown
Member

#1973 (comment) is unresolved, and my suggestion is to keep message at the very last, matching existing pattern in tracing and log. Or special case "message" attribute name like done by OTel Rust (https://github.com/open-telemetry/opentelemetry-rust/pull/2689/changes) with blessing from tracing maintainers.

@jmacd jmacd added this pull request to the merge queue Feb 6, 2026
@jmacd
Copy link
Copy Markdown
Contributor

jmacd commented Feb 6, 2026

I think we can try to improve the macros later, or change this stuff again -- what's important is we have both event name and optional message. (No more complaints from me!)

Merged via the queue into open-telemetry:main with commit a9fbb0e Feb 6, 2026
62 checks passed
@cijothomas
Copy link
Copy Markdown
Member

I think we can try to improve the macros later, or change this stuff again -- what's important is we have both event name and optional message. (No more complaints from me!)

As suggested here, we already had the ability to send optional message with special casing "message" attribute.
#1973 (comment)

github-merge-queue Bot pushed a commit that referenced this pull request Feb 6, 2026
Reverting #1973
Fixing the empty "" from our internal macros, that caused the
`message="user friendly message here"` from being omitted in stdout!

Taking
https://github.com/open-telemetry/otel-arrow/blob/main/rust/otap-dataflow/crates/controller/src/lib.rs#L668-L671
as example
```rust
otel_warn!(
                "core_affinity.set_failed",
                message = "Failed to set core affinity for pipeline thread. Performance may be less predictable."
            );
```

Before
```txt
2026-02-06T22:15:09.891Z  WARN  otap-df-controller::core_affinity.set_failed (crates/controller/src/lib.rs:668): 
```
(Missing message!)

After (i.e with this PR)
```txt
2026-02-06T22:11:19.095Z  WARN  otap-df-controller::core_affinity.set_failed (crates/controller/src/lib.rs:668): Failed to set core affinity for pipeline thread. Performance may be less predictable.
```
(Message is back)

"message" is already special cased in this repo, OTel Rust repo, and
`tracing` itself. Passing user friendly string as an attribute named
"message" is
*[faster](https://github.com/open-telemetry/opentelemetry-rust/pull/2001/changes)*
too!

Also, we avoid the less friendly syntax -
#1981 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update Rust code

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants