Skip to content
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

[API] Support for Logger::Enabled() is incomplete #2667

Open
marcalff opened this issue May 14, 2024 · 4 comments
Open

[API] Support for Logger::Enabled() is incomplete #2667

marcalff opened this issue May 14, 2024 · 4 comments
Labels
abi:version_2 Fix is available WITH_ABI_VERSION_2 breaking change API or ABI breaking change bug Something isn't working Stale triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@marcalff
Copy link
Member

  nostd::shared_ptr<opentelemetry::logs::Logger> logger = ...;

  if (logger->Enabled(severity)) {
  }

Logger::Enabled() always return false.

This is due to:

  inline bool Enabled(Severity severity) const noexcept
  {
    return static_cast<uint8_t>(severity) >= OPENTELEMETRY_ATOMIC_READ_8(&minimum_severity_);
  }

  mutable uint8_t minimum_severity_{kMaxSeverity};

The severity is kMaxSeverity by default, meaning all events are disabled by default.

There is a method to change the logger severity:

  void SetMinimumSeverity(uint8_t severity_or_max) noexcept
  {
    OPENTELEMETRY_ATOMIC_WRITE_8(&minimum_severity_, severity_or_max);
  }

but is it protected, and never called in the code base.

Any user code that checks for Enabled() can not emit logs.

Note that EmitLogRecord() helpers never honor the Enabled() flag either.

@marcalff marcalff added the bug Something isn't working label May 14, 2024
@github-actions github-actions bot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label May 14, 2024
@marcalff
Copy link
Member Author

To revisit once spec is merged:

@marcalff
Copy link
Member Author

Spec is now merged.

Adding a Enabled() method in the API is an ABI breaking change, to implement in ABI_VERSION_2 only.

@marcalff marcalff added abi:version_2 Fix is available WITH_ABI_VERSION_2 breaking change API or ABI breaking change triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 28, 2024
@marcalff
Copy link
Member Author

Copy link

github-actions bot commented Aug 1, 2024

This issue was marked as stale due to lack of activity.

@github-actions github-actions bot added the Stale label Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abi:version_2 Fix is available WITH_ABI_VERSION_2 breaking change API or ABI breaking change bug Something isn't working Stale triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

1 participant