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

NodeSDK constructor not using defaultAttributes from NodeSDKConfiguration #3717

Closed
zvickery opened this issue Apr 6, 2023 · 2 comments · Fixed by #3724
Closed

NodeSDK constructor not using defaultAttributes from NodeSDKConfiguration #3717

zvickery opened this issue Apr 6, 2023 · 2 comments · Fixed by #3724
Assignees
Labels
bug Something isn't working priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization

Comments

@zvickery
Copy link

zvickery commented Apr 6, 2023

What happened?

defaultAttributes setting in the NodeSDK constructor doesn't seem to do anything. Should it be there at all?

Steps to Reproduce

Initialize the SDK with default attributes set, like below:

const sdk = new NodeSDK({
    defaultAttributes: { ['testing']: '123' },
    ...
});

Expected Result

Provided attributes added to resulting auto-instrumented OTel Traces

Actual Result

Nothing happens

Additional Details

From the documentation and types I assumed we could set defaultAttributes to get extra attributes injected into the tracer. My intent is to add some attributes for searching across auto-instrumented traces. However this doesn't work because the code doesn't use defaultAttributes at all: https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-sdk-node/src/sdk.ts#L90

Generally speaking, there doesn't seem to be any other plumbing for this that I could see in the SDK. Is the defaultAttributes functionality supposed to work? Or is there another way to do this?

OpenTelemetry Setup Code

// I just invoke the NodeSDK constructor with standard setup code.  All is working well except I
// cannot add extra attributes to auto-instrumented traces

package.json

No response

Relevant log output

No response

@zvickery zvickery added bug Something isn't working triage labels Apr 6, 2023
@Flarna
Copy link
Member

Flarna commented Apr 12, 2023

I think it should be removed.
I assume this is some leftover because parts of OTel were based on OpenTracing/OpenCensus. As far as I remember default attributes have been replaced by resource attributes.

I removed something similar in OTLP exporters a while ago: #2991

@pichlermarc pichlermarc added priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization and removed triage labels Apr 12, 2023
@pichlermarc
Copy link
Member

Yep, that looks like a leftover, this can be done with Resource now.
I'll open a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants