chore: reviewing TODO comments

.github/workflows/test.yml

@@ -42,7 +42,7 @@ jobs:
           - '16'
           - '16.0'
           - '14'
-          - '14.17' # TODO: is this our actual min? same as elastic-apm-node
+          - '14.17'
 
     runs-on: ubuntu-latest
 

packages/opentelemetry-node/lib/environment.js

@@ -17,7 +17,6 @@
  * under the License.
  */
 
-// TODO: consider having a constants file
 const ELASTIC_SDK_VERSION = require('../package.json').version;
 const OTEL_SDK_VERSION =
     require('@opentelemetry/sdk-node/package.json').version;

packages/opentelemetry-node/lib/sdk.js

@@ -49,7 +49,6 @@ class ElasticNodeSDK extends NodeSDK {
 
         // Setup & fix some env
         setupEnvironment();
-        // TODO detect service name
 
         // - NodeSDK defaults to `TracerProviderWithEnvExporters` if neither
         //   `spanProcessor` nor `traceExporter` are passed in.
@@ -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
             ],
         };
 

packages/opentelemetry-node/start.js

@@ -22,19 +22,6 @@ const {ElasticNodeSDK} = require('./lib/sdk.js');
 
 const sdk = new ElasticNodeSDK();
 
-// TODO sdk shutdown: also SIGINT?
-// TODO sdk shutdown handlers: better log on err
-// TODO sdk shutdown: make these handlers configurable?
-//  - If so, could move these into sdk.js and it could use its logger for logging an error.
-//  - Note, only want luggite.warn over console.warn if errSerializer gets
-//    all attributes (console.warn shows more data for ECONNREFUSED)
-// TODO sdk shutdown beforeExit: skip this for Lambda (also Azure Fns?)
-// TODO sdh shutdown: call process.exit?
-// TODO: Whether we have a signal handler for sdk.shutdown() is debatable. It
-// 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.
-
 process.on('SIGTERM', async () => {
     try {
         await sdk.shutdown();

packages/opentelemetry-node/test/instr-winston.test.js

@@ -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
             const spans = col.sortedSpans;
             t.equal(spans.length, 1);
 

packages/opentelemetry-node/test/testutils.js

@@ -340,9 +340,6 @@ class TestCollector {
             });
         });
 
-        // TODO: for now we do not need any type of sorting
-        // To do so we need to take into account that each metric has a different
-        // property depending of the DataPointType (GAUGE, HISTOGRAM, ...) they have
         return metrics;
     }
 
@@ -361,7 +358,6 @@ class TestCollector {
             });
         });
 
-        // TODO: sorting required? Or is incoming order sufficient? s/logs/sortedLogs/ if actively sorting.
         return logs;
     }
 }