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

Fix OrderMap reference in log subsystem #1073

Merged
merged 3 commits into from
May 30, 2023

Conversation

lalitb
Copy link
Member

@lalitb lalitb commented May 23, 2023

Fixes #
Design discussion issue (if applicable) #

Changes

As part of PR #1061, the OrderMap module was moved from copentelemetry-api::trace to opentelemetry-api root. This PR ensures that log subsystem should refer to the OrderMap module from opentelemetry-api, and not from opentelemetry-api::trace. Also added the reexport in logs to ensure that existing usage (if any) is not broken.

Please provide a brief description of the changes here.

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@lalitb lalitb requested a review from a team May 23, 2023 05:19
@codecov
Copy link

codecov bot commented May 23, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (e9adef6) 50.9% compared to head (ecd5397) 50.9%.

❗ Current head ecd5397 differs from pull request most recent head f3096b1. Consider uploading reports for the commit f3096b1 to get more accurate results

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1073   +/-   ##
=====================================
  Coverage   50.9%   50.9%           
=====================================
  Files        165     165           
  Lines      19689   19689           
=====================================
  Hits       10025   10025           
  Misses      9664    9664           
Impacted Files Coverage Δ
opentelemetry-api/src/logs/mod.rs 0.0% <ø> (ø)
opentelemetry-api/src/logs/record.rs 0.0% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -13,6 +13,9 @@ pub use logger::{Logger, LoggerProvider};
pub use noop::NoopLoggerProvider;
pub use record::{AnyValue, LogRecord, LogRecordBuilder, Severity, TraceContext};

use std::collections::hash_map::RandomState;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is needed anymore? Is the only remaining value of this PR sorting the imports correctly?

Copy link
Member Author

@lalitb lalitb May 25, 2023

Choose a reason for hiding this comment

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

Good catch. Removed the import.

@TommyCpp TommyCpp merged commit 2e4d396 into open-telemetry:main May 30, 2023
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.

3 participants