-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Oracle Cloud Resource Detection Processor #42632
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
Oracle Cloud Resource Detection Processor #42632
Conversation
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
|
Will you be owning this code moving forward? |
|
@atoulme Yes! I, along with two of my colleagues at Oracle, @tonychoe and @raphaelfan can be the codeowners. We discussed ownership and project sponsorship with @dashpole (who volunteered to be the project sponsor) in this linked issue: |
|
Feel free to ping me on slack once tests are passing |
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
|
Moving to draft while you work out the issues, please mark ready for review when CI passes. |
|
Also wanted to mention that I tried to add myself and my Oracle teammates as codeowners in our |
|
@geekdave you can add yourself to the allowlist under cmd/githubgen/allowlist.txt one username per line if you would like to be codeowners ahead of joining as members. Or we can add you post facto. I have triggered the build. |
|
@dashpole please review |
|
@atoulme Thanks for the tip! If it's alright I will hold my breath and back away slowly from this PR's green CI status, and update the allowlist as a follow-up PR after-the-fact. 😉 |
| // Detect detects system metadata and returns a resource with the available ones | ||
| func (d *Detector) Detect(ctx context.Context) (resource pcommon.Resource, schemaURL string, err error) { | ||
| compute, err := d.provider.Metadata(ctx) | ||
| if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach is OK, but you may want to try and differentiate between the case where someone is not on oracle cloud (where return pcommon.NewResource(), "", nil is correct), and the case where someone is on oracle cloud, but the request failed (e.g. a timeout?). Currently, it is possible for a user to end up with missing resource attributes in the latter case.
dashpole
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feedback above is non-blocking, but I would recommend filing an issue to track fixing it if you choose not to address it in this PR.
|
@geekdave please file an issue or fix it here, and we can merge. |
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
Description
Implements a resource detection processor for Oracle Cloud Infrastructure (OCI) to enrich telemetry with environment metadata.
Uses semantic conventions for OCI: open-telemetry/semantic-conventions#1720
Link to tracking issue
Fixes #35091
Testing
Documentation
processor/resourcedetectionprocessor/README.md) to describe Oracle Cloud detection usage and configuration.