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

[BUILD] Fix references to trace namespace to be fully qualified #2422

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

bogdandrutu
Copy link
Member

Fixes #2417

@bogdandrutu bogdandrutu requested a review from a team December 1, 2023 01:10
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Copy link

codecov bot commented Dec 1, 2023

Codecov Report

Merging #2422 (b65cd95) into main (064fef0) will not change coverage.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2422   +/-   ##
=======================================
  Coverage   87.06%   87.06%           
=======================================
  Files         199      199           
  Lines        6079     6079           
=======================================
  Hits         5292     5292           
  Misses        787      787           
Files Coverage Δ
...pentelemetry/sdk/metrics/exemplar/reservoir_cell.h 58.83% <0.00%> (ø)
...ude/opentelemetry/sdk/metrics/data/exemplar_data.h 0.00% <0.00%> (ø)

Copy link
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

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

Thanks

Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

Thanks for the fix.

@marcalff marcalff merged commit 27a58cd into open-telemetry:main Dec 1, 2023
45 checks passed
@bogdandrutu bogdandrutu deleted the fix_2417 branch December 1, 2023 20:02
@qinyeli
Copy link

qinyeli commented Dec 1, 2023

@bogdandrutu Thanks for the fix!!
I noticed a similar issue in otlp_recordable/opentelemetry/exporters/otlp/otlp_recordable.h and opentelemetry/exporters/etw/etw_tracer.h as well. Can you help fix those as well?

bazel-out/aarch64-fastbuild/bin/external/io_opentelemetry_cpp/exporters/otlp/_virtual_includes/otlp_recordable/opentelemetry/exporters/otlp/otlp_recordable.h:51:18: error: 'opentelemetry::v1::exporter::trace::StatusCode' has not been declared
   51 |   void SetStatus(trace::StatusCode code, nostd::string_view description) noexcept override;
      |                  ^~~~~
bazel-out/aarch64-fastbuild/bin/external/io_opentelemetry_cpp/exporters/otlp/_virtual_includes/otlp_recordable/opentelemetry/exporters/otlp/otlp_recordable.h:51:8: error: 'void opentelemetry::v1::exporter::otlp::OtlpRecordable::SetStatus(int, opentelemetry::v1::nostd::string_view)' marked 'override', but does not override
   51 |   void SetStatus(trace::StatusCode code, nostd::string_view description) noexcept override;
      |        ^~~~~~~~~

I went through all "trace::" in the repository, and it seems that those not-fully-qualified namespace in api (eg) and those in .cc (eg) does not cause the same issue. However, I assume those in the .h files should be fixed.

@marcalff marcalff changed the title Fix references to trace namespace to be fully qualified [BUILD] Fix references to trace namespace to be fully qualified Dec 5, 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.

'SpanContext' is not a member of 'opentelemetry::v1::sdk::trace'
5 participants