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 #1729

Closed
wants to merge 2 commits into from

Conversation

rgc183
Copy link

@rgc183 rgc183 commented Nov 2, 2022

Fixes # (issue)

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

@rgc183 rgc183 requested a review from a team November 2, 2022 13:15
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 2, 2022

CLA Not Signed

@codecov
Copy link

codecov bot commented Nov 2, 2022

Codecov Report

Merging #1729 (d477e9d) into main (b8b715f) will decrease coverage by 0.02%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1729      +/-   ##
==========================================
- Coverage   85.79%   85.77%   -0.01%     
==========================================
  Files         171      171              
  Lines        5212     5212              
==========================================
- Hits         4471     4470       -1     
- Misses        741      742       +1     
Impacted Files Coverage Δ
sdk/src/trace/batch_span_processor.cc 90.63% <0.00%> (-0.78%) ⬇️

@lalitb
Copy link
Member

lalitb commented Nov 2, 2022

@rgc183 Thanks for the fix. You would need to sign the CLA before you can contribute. Also, need to fix the code format as described here - https:/open-telemetry/opentelemetry-cpp/blob/main/CONTRIBUTING.md#how-to-send-pull-requests

@marcalff
Copy link
Member

marcalff commented Nov 2, 2022

@rgc183 Thanks for the fix.

Please fix the clang-format (extra line), see from the build logs:

diff --git a/bazel/extra_deps.bzl b/bazel/extra_deps.bzl
index 1c1ddda..5b892e5 100644
--- a/bazel/extra_deps.bzl
+++ b/bazel/extra_deps.bzl
@@ -3,7 +3,6 @@ load("@com_github_jupp0r_prometheus_cpp//bazel:repositories.bzl", "prometheus_cp
 load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace")
 load("@rules_foreign_cc//foreign_cc:repositories.bzl", "rules_foreign_cc_dependencies")
 
-
 def opentelemetry_extra_deps():
     prometheus_cpp_repositories()
     bazel_skylib_workspace()

And please sign the CLA.

@lalitb
Copy link
Member

lalitb commented Nov 4, 2022

@rgc183 - let us know if you need help with formatting, or have issues with CLA signing?

@rgc183
Copy link
Author

rgc183 commented Nov 4, 2022

@lalitb Please give me sometime, I am working with my organization to get CLA approval and some sign-offs.

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 after CLA signed,

@marcalff marcalff added the pr:waiting-on-cla Waiting on CLA label Nov 21, 2022
@rgc183
Copy link
Author

rgc183 commented Nov 23, 2022

@lalitb As a corporate contributor, when I select my organization, there is list of people I see as CLA managers. I do not see how to add new person to send an email to. Also, the person mentioned in the list, I cannot locate him internally.
I have internal approval to contribute to this project.

@rgc183
Copy link
Author

rgc183 commented Nov 23, 2022

Created another MR: #1795 from my corporate account. Closing this one.

@rgc183 rgc183 closed this Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:waiting-on-cla Waiting on CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prometheus exporter gives build error
5 participants