-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(config): add resource detection parsing #6435
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
Changes from all commits
d5dff48
a069e5e
82639ab
89623f6
fd29d73
4111ccb
97b2d7f
3c9c238
8300978
3e6a8f7
965013c
76f01dc
ccadf11
dd5d7bc
9b3bdb0
bb6d2fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,31 +128,16 @@ export function getResourceDetectorsFromEnv(): Array<ResourceDetector> { | |
| export function getResourceDetectorsFromConfiguration( | ||
| config: ConfigurationModel | ||
| ): Array<ResourceDetector> { | ||
| // When updating this list, make sure to also update the section `resourceDetectors` on README. | ||
| const resourceDetectors = new Map<string, ResourceDetector>([ | ||
| [RESOURCE_DETECTOR_HOST, hostDetector], | ||
| [RESOURCE_DETECTOR_OS, osDetector], | ||
| [RESOURCE_DETECTOR_SERVICE_INSTANCE_ID, serviceInstanceIdDetector], | ||
| [RESOURCE_DETECTOR_PROCESS, processDetector], | ||
| [RESOURCE_DETECTOR_ENVIRONMENT, envDetector], | ||
| ]); | ||
|
|
||
| const resourceDetectorsFromConfig = config.node_resource_detectors ?? []; | ||
|
|
||
| if (resourceDetectorsFromConfig.includes('all')) { | ||
| return [...resourceDetectors.values()].flat(); | ||
| } | ||
|
|
||
| if (resourceDetectorsFromConfig.includes('none')) { | ||
| return []; | ||
| } | ||
|
|
||
| return resourceDetectorsFromConfig.flatMap(detector => { | ||
| const resourceDetector = resourceDetectors.get(detector); | ||
| if (!resourceDetector) { | ||
| diag.warn(`Invalid resource detector "${detector}" specified`); | ||
| } | ||
| return resourceDetector || []; | ||
| const detectors = config.resource?.['detection/development']?.detectors ?? []; | ||
|
|
||
| return detectors.flatMap(detector => { | ||
| const result: ResourceDetector[] = []; | ||
| if (detector.host != null) result.push(hostDetector); | ||
| if (detector.os != null) result.push(osDetector); | ||
| if (detector.process != null) result.push(processDetector); | ||
| if (detector.service != null) result.push(serviceInstanceIdDetector); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are allowed to have a breaking change in v3. I wonder if we want to change our (currently experimental, I think?) (I'm not sure if the intent is that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree this is worth discussing, but feels like a separate concern from this PR. Happy to open a follow-up issue to track renaming
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Absolutely a separate PR kind of thing.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the idea of a To answer your other question:
I think being handled by the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks both - I like the idea of replacing I opened the follow-up issue to track doing that
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW, I spent some time looking at OTel Java and Python implementations, and ... gave up after a little bit. Python doesn't have anything for service.instance.id I don't think, yet. OTel Java has a It also has a Anyway, clarity to seek on another day.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the notes @trentm - I've been helping out on the Java implementation and am leading the Python implementation too (which is still very early). I'd like to consolidate on a consistent pattern too and will keep an eye on it the linked issue 👍🏻 |
||
| if (detector.env != null) result.push(envDetector); | ||
| return result; | ||
| }); | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.