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

diag.error about unknown OTEL_NODE_RESOURCE_DETECTORS entry should be diag.warn #4882

Closed
trentm opened this issue Jul 26, 2024 · 1 comment · Fixed by #5034
Closed

diag.error about unknown OTEL_NODE_RESOURCE_DETECTORS entry should be diag.warn #4882

trentm opened this issue Jul 26, 2024 · 1 comment · Fixed by #5034
Labels
bug Something isn't working good first issue Good for newcomers pkg:sdk-node priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization

Comments

@trentm
Copy link
Contributor

trentm commented Jul 26, 2024

if (!resourceDetector) {
diag.error(
`Invalid resource detector "${detector}" specified in the environment variable OTEL_NODE_RESOURCE_DETECTORS`
);
}

Currently it is a diag.error, but I think it should be a diag.warn.

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#enum-value says:

if the user provides a value the implementation does not recognize, the implementation MUST generate a warning and gracefully ignore the setting.

Originally posted by @trentm in #4879 (comment)


There is a similar diag.error in auto-instrumentations-node that I think should be a diag.warn as well:
https://github.com/open-telemetry/opentelemetry-js-contrib/blob/6e8989de4d60c32bc9b3e3ef760e92c68ae7f491/metapackages/auto-instrumentations-node/src/utils.ts#L280

@trentm trentm added the good first issue Good for newcomers label Jul 26, 2024
RichardChukwu added a commit to RichardChukwu/opentelemetry-js that referenced this issue Oct 2, 2024
…CTORS values as per issue open-telemetry#4882.

I referenced the OpenTelemetry specification document, which clarified that diag.warn should be used for unrecognized values that don't critically break the system.
@pichlermarc pichlermarc added bug Something isn't working priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization pkg:sdk-node labels Oct 3, 2024
@pichlermarc
Copy link
Member

marking as bug/p4 (incorrect log level)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers pkg:sdk-node priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization
Projects
None yet
2 participants