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

[Exporter] add fix for prometheus exporter build #1795

Merged
merged 2 commits into from
Nov 24, 2022

Conversation

rochaudhari
Copy link
Contributor

@rochaudhari rochaudhari commented Nov 23, 2022

Signed-off-by: Roshan Chaudhari rochaudhari@nvidia.com

Fixes #1728

For prometheus exporter, required dependencies are not loaded in repository.bzl. When Opentelemetry is imported in some other project, these dependencies are missing.

@rochaudhari rochaudhari requested a review from a team November 23, 2022 07:36
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 23, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

bazel/extra_deps.bzl Outdated Show resolved Hide resolved
@lalitb
Copy link
Member

lalitb commented Nov 23, 2022

As of now, CI build is failing for #1791, this PR can be merged once the build is fixed.

@rochaudhari rochaudhari force-pushed the bazel_fix branch 2 times, most recently from 216bdfd to bcc4932 Compare November 23, 2022 08:33
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.

LGTM, thanks for the fix.

@codecov
Copy link

codecov bot commented Nov 23, 2022

Codecov Report

Merging #1795 (9ae4a9e) into main (3f0eee6) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1795   +/-   ##
=======================================
  Coverage   85.71%   85.71%           
=======================================
  Files         171      171           
  Lines        5240     5240           
=======================================
  Hits         4491     4491           
  Misses        749      749           

INSTALL.md Outdated Show resolved Hide resolved
@lalitb
Copy link
Member

lalitb commented Nov 24, 2022

@rochaudhari - CI test is failing for code formatting. Can you fix the formatting as suggested here - https://github.com/open-telemetry/opentelemetry-cpp/blob/main/CONTRIBUTING.md#how-to-send-pull-requests

Signed-off-by: Roshan Chaudhari rochaudhari@nvidia.com
@rochaudhari
Copy link
Contributor Author

@rochaudhari - CI test is failing for code formatting. Can you fix the formatting as suggested here - https://github.com/open-telemetry/opentelemetry-cpp/blob/main/CONTRIBUTING.md#how-to-send-pull-requests

Strange! After running format.sh as well, the file looks same. I updated the patch again.

@rochaudhari
Copy link
Contributor Author

@lalitb Can we merge this?
When is the next release planned?

@lalitb lalitb merged commit 8bfb9a3 into open-telemetry:main Nov 24, 2022
@esigo
Copy link
Member

esigo commented Nov 24, 2022

@lalitb Can we merge this? When is the next release planned?

@rochaudhari, I'll prepare the release this weekend.

yxue pushed a commit to yxue/opentelemetry-cpp that referenced this pull request Dec 5, 2022
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.

Prometheus exporter gives build error
5 participants