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

Fixed example resource provider fully qualified class name #5235

Merged
merged 1 commit into from
Feb 17, 2023
Merged

Fixed example resource provider fully qualified class name #5235

merged 1 commit into from
Feb 17, 2023

Conversation

cheempz
Copy link
Contributor

@cheempz cheempz commented Feb 17, 2023

Related to #4748, updated to the new package name.

@cheempz cheempz requested a review from a team February 17, 2023 01:57
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 17, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: cheempz / name: Lin Lin (18834ab)

@codecov
Copy link

codecov bot commented Feb 17, 2023

Codecov Report

Base: 91.08% // Head: 91.08% // No change to project coverage 👍

Coverage data is based on head (18834ab) compared to base (2a30803).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #5235   +/-   ##
=========================================
  Coverage     91.08%   91.08%           
  Complexity     4876     4876           
=========================================
  Files           549      549           
  Lines         14421    14421           
  Branches       1372     1372           
=========================================
  Hits          13136    13136           
  Misses          891      891           
  Partials        394      394           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -238,7 +238,7 @@ For example, if you don't want to expose the name of the operating system throug
can pass the following JVM argument:

```
-Dotel.java.disabled.resource.providers=io.opentelemetry.sdk.extension.resources.OsResourceProvider
-Dotel.java.disabled.resource.providers=io.opentelemetry.instrumentation.resources.OsResourceProvider
Copy link
Contributor

Choose a reason for hiding this comment

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

hrm. maybe we should remove this example altogether, since it depends on the instrumentation repository. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, well the section is prefaced with

If you are using the ResourceProvider SPI (many instrumentation agent distributions include this automatically)

and the example did help me eventually find my way heh, but happy to just remove it too to reduce confusion.

Copy link
Contributor

Choose a reason for hiding this comment

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

let's keep it for now.

@jkwatson
Copy link
Contributor

@cheempz we can't accept this PR until the CLA has been signed.

@cheempz
Copy link
Contributor Author

cheempz commented Feb 17, 2023

OK not-so-EasyCLA finally worked after several tries.

@jkwatson jkwatson merged commit ce18841 into open-telemetry:main Feb 17, 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.

2 participants