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 a resourceDetectors configuration to the SDK #3210

Closed
svrnm opened this issue Aug 29, 2022 · 2 comments · Fixed by #3212
Closed

Add a resourceDetectors configuration to the SDK #3210

svrnm opened this issue Aug 29, 2022 · 2 comments · Fixed by #3212
Assignees

Comments

@svrnm
Copy link
Member

svrnm commented Aug 29, 2022

Is your feature request related to a problem? Please describe.

Since we had an internal need for that, I wanted to write down a small sample where I use the NodeSDK with some additional resource detectors (docker, aws). Unfortunately I can not add them without a lot of additional code.

Eventually I wanted to add this to the JS docs over at opentelemetry.io so having something user-friendly is what I am looking for

Describe the solution you'd like

I found this list in opentelemetry-sdk-node package and I was hoping that it would be feasible to make this list extendible with an interface that looks like this:

import { dockerCGroupV1Detector } from '@opentelemetry/resource-detector-docker'
import { awsEc2Detector } from '@opentelemetry/resource-detector-aws'

const sdk = new opentelemetry.NodeSDK({
   traceExporter: new opentelemetry.tracing.ConsoleSpanExporter(),
   instrumentations: [getNodeAutoInstrumentations()],
   resourceDetectors: [dockerCGroupV1Detector, awsEc2Detector]
});

Describe alternatives you've considered

What I can do right now is either overwriting the OTEL_RESOURCE_ATTRIBUTES with the attributes I want or I can do something like this (via this thread from @osherv:

const detectedResources = await detectResources({
    detectors: [envDetector, processDetector, dockerCGroupV1Detector, awsEc2Detector],
  });

  const myResource = new Resource({
    [SemanticResourceAttributes.SERVICE_NAME]: "serviceName",
    ["my.custom.attr"]: "someValue",
  });

const sdk = new opentelemetry.NodeSDK({
  traceExporter: new opentelemetry.tracing.ConsoleSpanExporter(),
  instrumentations: [getNodeAutoInstrumentations()],
   resource:  myResource.merge(detectedResources),
  autoDetectResources: false
});

Additional context

@osherv agreed to pick this up, if @open-telemetry/javascript-maintainers agree that this would be a good addition to the NodeSDK

Side Note: Eventually it would be cool to have a package like @opentelemetry/auto-instrumentations-node for resource detection that would allow something like resourceDetectors: getNodeResourceDetectors()

@osherv
Copy link
Member

osherv commented Aug 29, 2022

That sounds like a handy feature, and I'll be happy to implement that. Assign to me if the issue gets approved :).

@legendecas
Copy link
Member

I'd agree it is a good addition. Assigned to @osherv, thank you for standing up for this.

osherv added a commit to epsagon/opentelemetry-js that referenced this issue Aug 30, 2022
osherv added a commit to epsagon/opentelemetry-js that referenced this issue Aug 30, 2022
osherv added a commit to epsagon/opentelemetry-js that referenced this issue Aug 30, 2022
osherv added a commit to epsagon/opentelemetry-js that referenced this issue Aug 30, 2022
osherv added a commit to epsagon/opentelemetry-js that referenced this issue Aug 30, 2022
osherv added a commit to epsagon/opentelemetry-js that referenced this issue Aug 30, 2022
osherv added a commit to epsagon/opentelemetry-js that referenced this issue Aug 31, 2022
osherv added a commit to epsagon/opentelemetry-js that referenced this issue Aug 31, 2022
osherv added a commit to epsagon/opentelemetry-js that referenced this issue Aug 31, 2022
legendecas pushed a commit that referenced this issue Sep 2, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* feat(node-sdk): added resourceDetectors option to NodeSDK #3210

* feat(node-sdk): fixed markdown lint #3210

* feat(node-sdk): fixed types lint #3210

* feat(node-sdk): added resourceDetectors option to NodeSDK #3210

* feat(node-sdk): added resourceDetectors option to NodeSDK #3210

* feat(node-sdk): updated proto subproject commit NodeSDK #3210

* feat(node-sdk): returned protobuf repos to v0.18.0 #3210

* Update experimental/packages/opentelemetry-sdk-node/README.md

Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>

* feat(node-sdk): added breaking changelog #3210

* feat(node-sdk): added breaking changelog #3210

* Update experimental/CHANGELOG.md

Co-authored-by: Gerhard Stöbich <deb2001-github@yahoo.de>

Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
Co-authored-by: Gerhard Stöbich <deb2001-github@yahoo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants