chore: reviewing TODO comments #129
Conversation
| @@ -49,7 +49,6 @@ class ElasticNodeSDK extends NodeSDK { | |||
|
|
|||
| // Setup & fix some env | |||
| setupEnvironment(); | |||
| // TODO detect service name | |||
There was a problem hiding this comment.
Per https://github.com/open-telemetry/semantic-conventions/blob/main/docs/attributes-registry/service.md
If the value was not specified, SDKs MUST fallback to unknown_service: concatenated with process.executable.name, e.g. unknown_service:bash. If process.executable.name is not available, the value MUST be set to unknown_service.
NodeSDK is already handling those MUST requirements. I'd hesitate to diverge from that by looking around in nearby package.json files. We could later add a feature request issue for this if someone wants/needs it.
| @@ -62,7 +61,6 @@ class ElasticNodeSDK extends NodeSDK { | |||
| processDetectorSync, | |||
| // hostDetectorSync is not currently in the OTel default, but may be added | |||
| hostDetectorSync, | |||
| // TODO cloud/container detectors by default | |||
| // definitely *can* change program behaviour. Let's reconsider. | ||
| // Note: See https://github.com/open-telemetry/opentelemetry-js/issues/1521 | ||
| // for some thoughts on automatic handling to shutdown the SDK. | ||
|
|
| @@ -40,8 +40,6 @@ const testFixtures = [ | |||
| recs = stdout.trim().split(/\n/g).map(JSON.parse); | |||
| }, | |||
| checkTelemetry: (t, col) => { | |||
| // We expect telemetry like this: | |||
| // TODO: waiting for log sending from https://github.com/open-telemetry/opentelemetry-js-contrib/pull/1837 to be in an otel-js-contrib release | |||
There was a problem hiding this comment.
Already have this. Forgot to remove this TODO comment in the last update to this test file.
| @@ -361,7 +358,6 @@ class TestCollector { | |||
| }); | |||
| }); | |||
|
|
|||
| // TODO: sorting required? Or is incoming order sufficient? s/logs/sortedLogs/ if actively sorting. | |||
There was a problem hiding this comment.
The TODOs removed here didn't add any value. If we need sorting of metrics and logs on the TestCollector, we'll come here to add it.
No description provided.