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

Add "using instrumentation libraries" for cpp #4388

Merged
merged 19 commits into from
May 24, 2024
Merged

Conversation

devc007
Copy link
Contributor

@devc007 devc007 commented Apr 29, 2024

Looking for the review to make this commit better.

@devc007 devc007 requested review from a team as code owners April 29, 2024 20:21
Copy link

linux-foundation-easycla bot commented Apr 29, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Member

@theletterf theletterf left a comment

Choose a reason for hiding this comment

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

Thanks! A first pass.

content/en/docs/languages/cpp/library.md Outdated Show resolved Hide resolved
content/en/docs/languages/cpp/library.md Outdated Show resolved Hide resolved
content/en/docs/languages/cpp/library.md Outdated Show resolved Hide resolved
content/en/docs/languages/cpp/library.md Outdated Show resolved Hide resolved
content/en/docs/languages/cpp/library.md Outdated Show resolved Hide resolved
content/en/docs/languages/cpp/library.md Outdated Show resolved Hide resolved
devc007 and others added 6 commits April 30, 2024 14:54
Co-authored-by: Fabrizio Ferri-Benedetti <fferribenedetti@splunk.com>
Co-authored-by: Fabrizio Ferri-Benedetti <fferribenedetti@splunk.com>
Co-authored-by: Fabrizio Ferri-Benedetti <fferribenedetti@splunk.com>
Co-authored-by: Fabrizio Ferri-Benedetti <fferribenedetti@splunk.com>
Co-authored-by: Fabrizio Ferri-Benedetti <fferribenedetti@splunk.com>
Co-authored-by: Fabrizio Ferri-Benedetti <fferribenedetti@splunk.com>
@theletterf
Copy link
Member

/fix:all

@opentelemetrybot
Copy link
Contributor

Co-authored-by: Severin Neumann <neumanns@cisco.com>
Copy link
Member

@svrnm svrnm 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 taking this challenge @devc007!

@devc007
Copy link
Contributor Author

devc007 commented Apr 30, 2024

Thanks for taking this challenge @devc007!

thank you @svrnm. waiting for merging this pull request and want to learn more about OpenTelemetry as a user and also want to contribute more on the project.

@svrnm
Copy link
Member

svrnm commented Apr 30, 2024

@open-telemetry/cpp-approvers please take a look

Co-authored-by: Severin Neumann <neumanns@cisco.com>
@svrnm svrnm added the sig-approval-missing Co-owning SIG didn't provide an approval label Apr 30, 2024
@devc007 devc007 requested a review from svrnm April 30, 2024 14:03
@devc007
Copy link
Contributor Author

devc007 commented May 3, 2024

@svrnm @theletterf sorry I don't know how to use local path, please can you help me

@theletterf
Copy link
Member

@devc007 Hmmm. What do you need the "local path" for?

@marcalff marcalff changed the title added "using instrumentationo libraries" for cpp added "using instrumentation libraries" for cpp May 6, 2024
@marcalff
Copy link
Member

marcalff commented May 6, 2024

@devc007 Hmmm. What do you need the "local path" for?

This is to fix the following errors in CI:

WARNINGs or ERRORs found in build log:
WARN  docs/languages/cpp/library.md: use a local path, not an external URL, for the following reference to a site local page: https://opentelemetry.io/docs/concepts/instrumentation/libraries/
WARN  docs/languages/cpp/library.md: use a local path, not an external URL, for the following reference to a site local page: https://opentelemetry.io/docs/specs/otel/glossary/#instrumentation-library
WARN  docs/languages/cpp/library.md: use a local path, not an external URL, for the following reference to a site local page: https://opentelemetry.io/ecosystem/registry/?language=cpp&component=instrumentation
WARN  docs/languages/cpp/library.md: use a local path, not an external URL, for the following reference to a site local page: https://opentelemetry.io/docs/languages/cpp/instrumentation/
WARN  docs/languages/cpp/library.md: use a local path, not an external URL, for the following reference to a site local page: https://opentelemetry.io/docs/languages/cpp/exporters/

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 documentation.

Please fix the URL reported by CI to use local path (../../somewhere/else) instead of an absolute path (https:://same-website/full/path/to/somewhere/else).

Looks good otherwise.

@marcalff marcalff changed the title added "using instrumentation libraries" for cpp Add "using instrumentation libraries" for cpp May 6, 2024
@svrnm
Copy link
Member

svrnm commented May 6, 2024

@devc007 best is to use the absolute path for local path, so /docs/languages/cpp/exporters/ should work in one of the examples

@svrnm
Copy link
Member

svrnm commented May 7, 2024

========================================================================
running in concurrent mode, this is experimental
docs/languages/cpp/library/index.html
  target does not exist --- docs/languages/cpp/library/index.html --> docs/concepts/instrumentation/libraries/
  target does not exist --- docs/languages/cpp/library/index.html --> docs/specs/otel/glossary/#instrumentation-library
  target does not exist --- docs/languages/cpp/library/index.html --> ecosystem/registry/?language=cpp&component=instrumentation
  target does not exist --- docs/languages/cpp/library/index.html --> docs/languages/cpp/instrumentation/
  target does not exist --- docs/languages/cpp/library/index.html --> docs/languages/cpp/exporters/
========================================================================

You might miss some leading slashes in your links

@svrnm
Copy link
Member

svrnm commented May 7, 2024

@marcalff minus the URL issues, is this PR good to go from @open-telemetry/cpp-approvers perspective?

@devc007
Copy link
Contributor Author

devc007 commented May 17, 2024

@theletterf @svrnm @marcalff Last time the URl's were creating problem and I have commited changes. Is there anything I need to improve?
I have revisited all the commits and incapable of finding any improvement by my own.

@svrnm
Copy link
Member

svrnm commented May 17, 2024

@theletterf @svrnm @marcalff Last time the URl's were creating problem and I have commited changes. Is there anything I need to improve? I have revisited all the commits and incapable of finding any improvement by my own.

apologies for not following up earlier! The links are fixed, so I think we are good, @open-telemetry/cpp-approvers any concerns with merging?

@svrnm
Copy link
Member

svrnm commented May 17, 2024

/fix:format

@opentelemetrybot
Copy link
Contributor

You triggered fix:format action run at https://github.com/open-telemetry/opentelemetry.io/actions/runs/9131113899

@svrnm
Copy link
Member

svrnm commented May 22, 2024

There are some strange bugs with EasyCLA:

communitybridge/easycla#4329

@jarias-lfx
Copy link

/easycla

@theletterf theletterf requested a review from marcalff May 23, 2024 16:40
@svrnm svrnm dismissed marcalff’s stale review May 24, 2024 15:55

changes have been addressed

@svrnm svrnm merged commit 924b2be into open-telemetry:main May 24, 2024
15 checks passed
@svrnm
Copy link
Member

svrnm commented May 24, 2024

Thank you for your contribution and patience @devc007

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig:cpp sig-approval-missing Co-owning SIG didn't provide an approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants