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

Set up for opentelemetry_proto_grpc_lib_type being shared #3136

Closed
wants to merge 2 commits into from

Conversation

tjcw
Copy link
Contributor

@tjcw tjcw commented Nov 11, 2024

Caution, this patch may cause one of the tests to go into a loop

Fixes #3126
Buildability for AIX

Changes

cmake/opentelemetry-proto.cmake to make opentelemetry_proto_grpc library link with gRPC::grpc++ if building shared

This PR split off from #3127

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

Caution, this patch may cause one of the tests to go into a loop
@tjcw tjcw requested a review from a team as a code owner November 11, 2024 09:09
Copy link

linux-foundation-easycla bot commented Nov 11, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link

netlify bot commented Nov 11, 2024

Deploy Preview for opentelemetry-cpp-api-docs canceled.

Name Link
🔨 Latest commit e8e31ec
🔍 Latest deploy log https://app.netlify.com/sites/opentelemetry-cpp-api-docs/deploys/67333640297f2100089083b7

Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.85%. Comparing base (497eaf4) to head (e8e31ec).
Report is 161 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3136      +/-   ##
==========================================
+ Coverage   87.12%   87.85%   +0.73%     
==========================================
  Files         200      195       -5     
  Lines        6109     6136      +27     
==========================================
+ Hits         5322     5390      +68     
+ Misses        787      746      -41     

see 100 files with indirect coverage changes

@DerrickLFX
Copy link

/easycla

Copy link
Member

@owent owent left a comment

Choose a reason for hiding this comment

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

LGTM and 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.

The following CI:

CMake test (build shared libraries with otlp-exporter and static gRPC)

systematically fails on this test:

    Start  44: exporter.otlp.OtlpGrpcExporterFactoryTest.BuildTest

which hangs for up to 6 hours, and is then killed by the github infrastructure.

No explanations for the root cause at this point, but evidence from the recent CI runs in the opentelemetry-cpp repository seem to point at this change, as other branches do not show this.

It could be that this change is correct and exposed an existing bug elsewhere, or that somehow this change is incorrect / incomplete.

In any case, we need this resolved before merging, to avoid breaking CI.

@tjcw
Copy link
Contributor Author

tjcw commented Nov 12, 2024

Asking a colleague, it seems we no longer use the function that is set up by this PR. So I can close it. Please reopen another matching PR if you want to investigate why it causes the CI test to go into a loop.

@tjcw tjcw closed this Nov 12, 2024
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.

Build on IBM AIX
4 participants