Maintenance on otel platform (#6)

* Update demo and instrumentation for semantic updates * Remove some detectors under Deno Deploy * Fine-tune detector lists * Update Deno versions in CI
cloudydeno · May 17, 2024 · ea0e851 · ea0e851
1 parent 6f90631
commit ea0e851
Show file tree

Hide file tree

Showing 7 changed files with 132 additions and 71 deletions.
diff --git a/.github/workflows/deno-ci.yml b/.github/workflows/deno-ci.yml
@@ -13,10 +13,10 @@ jobs:
     strategy:
       matrix:
         deno-version:
-        - v1.31
         - v1.35
         - v1.37
         - v1.39
+        - v1.43
         - canary
       fail-fast: false # run each branch to completion
 

diff --git a/demo.ts b/demo.ts
@@ -1,7 +1,7 @@
 #!/usr/bin/env -S deno run --watch --allow-read --allow-sys=hostname,osRelease --allow-env --allow-net --allow-run=uptime,sleep,ping
 import { metrics, trace, ValueType } from "./opentelemetry/api.js";
 import { logs } from "./opentelemetry/api-logs.js";
-import { SemanticAttributes } from "./opentelemetry/semantic-conventions.js";
+import { SEMATTRS_HTTP_METHOD } from "./opentelemetry/semantic-conventions.js";
 
 import { DenoTelemetrySdk } from './sdk.ts'
 import { httpTracer } from "./instrumentation/http-server.ts";
@@ -30,8 +30,8 @@ async function handler(req: Request): Promise<Response> {
   const url = new URL(req.url);
   console.log(req.method, url.pathname);
 
-  test3.add(1, {[SemanticAttributes.HTTP_METHOD]: req.method});
-  test2.record(50+Math.round(Math.random()*25), {[SemanticAttributes.HTTP_METHOD]: req.method});
+  test3.add(1, {[SEMATTRS_HTTP_METHOD]: req.method});
+  test2.record(50+Math.round(Math.random()*25), {[SEMATTRS_HTTP_METHOD]: req.method});
 
   logger.emit({
     body: 'hello world',

diff --git a/instrumentation/deno-kv.ts b/instrumentation/deno-kv.ts
@@ -4,12 +4,6 @@
 // So we just don't know when additional batches are pulled.
 // (I guess we could monitor the number of results and compare to the desired batch size, but, ew)
 
-// Also, Deno KV is --unstable (and probably will be for a bit?)
-// so this file doesn't typecheck without --unstable
-// Should be ok since Deno doesn't typecheck remote modules by default anymore.
-// well, whatever
-// @ts-nocheck
-
 import { Span, SpanKind } from "../opentelemetry/api.js";
 import {
   isWrapped,

diff --git a/instrumentation/fetch.ts b/instrumentation/fetch.ts
@@ -1,4 +1,5 @@
 // copy of https://github.com/open-telemetry/opentelemetry-js/blob/ecb5ebe86eebc5a1880041b2523adf5f6d022282/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts
+// but with some fixes and improvements for backend use
 /*
  * Copyright The OpenTelemetry Authors
  *
@@ -34,15 +35,32 @@ export enum AttributeNames {
   HTTP_STATUS_TEXT = 'http.status_text',
 }
 
+import * as core from "../opentelemetry/core.js";
 import {
   isWrapped,
   InstrumentationBase,
   InstrumentationConfig,
   safeExecuteInTheMiddle,
 } from "../opentelemetry/instrumentation.js";
-import * as core from "../opentelemetry/core.js";
-import { SemanticAttributes } from "../opentelemetry/semantic-conventions.js";
-import { Context, context, HrTime, propagation, Span, SpanKind, SpanStatusCode, trace } from "../opentelemetry/api.js";
+import {
+  SEMATTRS_HTTP_HOST,
+  SEMATTRS_HTTP_METHOD,
+  SEMATTRS_HTTP_ROUTE,
+  SEMATTRS_HTTP_SCHEME,
+  SEMATTRS_HTTP_STATUS_CODE,
+  SEMATTRS_HTTP_URL,
+  SEMATTRS_HTTP_USER_AGENT,
+} from "../opentelemetry/semantic-conventions.js";
+import {
+  context,
+  Context,
+  HrTime,
+  propagation,
+  Span,
+  SpanKind,
+  SpanStatusCode,
+  trace,
+} from "../opentelemetry/api.js";
 
 export interface FetchCustomAttributeFunction {
   (
@@ -95,17 +113,17 @@ export class FetchInstrumentation extends InstrumentationBase {
     response: FetchResponse
   ): void {
     const parsedUrl = new URL(response.url);
-    span.setAttribute(SemanticAttributes.HTTP_STATUS_CODE, response.status);
+    span.setAttribute(SEMATTRS_HTTP_STATUS_CODE, response.status);
     if (response.statusText != null) {
       span.setAttribute(AttributeNames.HTTP_STATUS_TEXT, response.statusText);
     }
-    span.setAttribute(SemanticAttributes.HTTP_HOST, parsedUrl.host);
-    span.setAttribute(SemanticAttributes.HTTP_ROUTE, parsedUrl.hostname); // Datadog likes having this
+    span.setAttribute(SEMATTRS_HTTP_HOST, parsedUrl.host);
+    span.setAttribute(SEMATTRS_HTTP_ROUTE, parsedUrl.hostname); // Datadog likes having this
     span.setAttribute(
-      SemanticAttributes.HTTP_SCHEME,
+      SEMATTRS_HTTP_SCHEME,
       parsedUrl.protocol.replace(':', '')
     );
-    span.setAttribute(SemanticAttributes.HTTP_USER_AGENT, navigator.userAgent);
+    span.setAttribute(SEMATTRS_HTTP_USER_AGENT, navigator.userAgent);
 
     if (response.status >= 500) {
       span.setStatus({
@@ -155,8 +173,8 @@ export class FetchInstrumentation extends InstrumentationBase {
       kind: SpanKind.CLIENT,
       attributes: {
         [AttributeNames.COMPONENT]: this.moduleName,
-        [SemanticAttributes.HTTP_METHOD]: method,
-        [SemanticAttributes.HTTP_URL]: url,
+        [SEMATTRS_HTTP_METHOD]: method,
+        [SEMATTRS_HTTP_URL]: url,
       },
     });
   }

diff --git a/instrumentation/http-server.ts b/instrumentation/http-server.ts
@@ -1,9 +1,24 @@
-import { NetTransportValues, SemanticAttributes } from "../opentelemetry/semantic-conventions.js";
+import {
+  NETTRANSPORTVALUES_IP_TCP,
+  SEMATTRS_HTTP_HOST,
+  SEMATTRS_HTTP_METHOD,
+  SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED,
+  SEMATTRS_HTTP_ROUTE,
+  SEMATTRS_HTTP_SCHEME,
+  SEMATTRS_HTTP_STATUS_CODE,
+  SEMATTRS_HTTP_URL,
+  SEMATTRS_HTTP_USER_AGENT,
+  SEMATTRS_NET_HOST_NAME,
+  SEMATTRS_NET_PEER_IP,
+  SEMATTRS_NET_PEER_PORT,
+  SEMATTRS_NET_TRANSPORT,
+} from "../opentelemetry/semantic-conventions.js";
 import {
   metrics,
   propagation,
   ROOT_CONTEXT,
   SpanKind,
+  SpanStatusCode,
   trace,
   ValueType,
   type TextMapGetter,
@@ -30,9 +45,9 @@ export function httpTracer(inner: Deno.ServeHandler, opts?: {
     const url = new URL(req.url);
 
     const reqMetricAttrs = {
-      [SemanticAttributes.HTTP_SCHEME]: url.protocol.split(':')[0],
-      [SemanticAttributes.HTTP_METHOD]: req.method,
-      [SemanticAttributes.NET_HOST_NAME]: url.host, // not sure why metrics spec wants it this way
+      [SEMATTRS_HTTP_SCHEME]: url.protocol.split(':')[0],
+      [SEMATTRS_HTTP_METHOD]: req.method,
+      [SEMATTRS_NET_HOST_NAME]: url.host, // not sure why metrics spec wants it this way
     };
     inflightMetric.add(1, reqMetricAttrs);
 
@@ -43,28 +58,28 @@ export function httpTracer(inner: Deno.ServeHandler, opts?: {
     return tracer.startActiveSpan(`${req.method} ${url.pathname}`, {
       kind: SpanKind.SERVER,
       attributes: {
-        [SemanticAttributes.HTTP_SCHEME]: url.protocol.split(':')[0],
-        [SemanticAttributes.HTTP_METHOD]: req.method,
-        [SemanticAttributes.HTTP_URL]: req.url,
-        [SemanticAttributes.HTTP_HOST]: url.host,
-        [SemanticAttributes.HTTP_USER_AGENT]: req.headers.get('user-agent') ?? undefined,
-        [SemanticAttributes.HTTP_ROUTE]: url.pathname, // for datadog
+        [SEMATTRS_HTTP_SCHEME]: url.protocol.split(':')[0],
+        [SEMATTRS_HTTP_METHOD]: req.method,
+        [SEMATTRS_HTTP_URL]: req.url,
+        [SEMATTRS_HTTP_HOST]: url.host,
+        [SEMATTRS_HTTP_USER_AGENT]: req.headers.get('user-agent') ?? undefined,
+        [SEMATTRS_HTTP_ROUTE]: url.pathname, // for datadog
         // 'http.request_content_length': '/http/request/size',
       },
     }, ctx, async (serverSpan) => {
       try {
 
         if (connInfo.remoteAddr.transport == 'tcp') {
           serverSpan.setAttributes({
-            [SemanticAttributes.NET_TRANSPORT]: NetTransportValues.IP_TCP,
-            [SemanticAttributes.NET_PEER_IP]: connInfo.remoteAddr.hostname,
-            [SemanticAttributes.NET_PEER_PORT]: connInfo.remoteAddr.port,
+            [SEMATTRS_NET_TRANSPORT]: NETTRANSPORTVALUES_IP_TCP,
+            [SEMATTRS_NET_PEER_IP]: connInfo.remoteAddr.hostname,
+            [SEMATTRS_NET_PEER_PORT]: connInfo.remoteAddr.port,
             ['net.sock.family']: connInfo.remoteAddr.hostname.includes(':') ? 'inet6' : 'inet',
           })
         // Unix sockets are currently behind --unstable so we can't just ref Deno.ServeUnixHandler today
         // } else if (connInfo.localAddr.transport == 'unix' || connInfo.localAddr.transport == 'unixpacket') {
         //   serverSpan.setAttributes({
-        //     [SemanticAttributes.NET_TRANSPORT]: NetTransportValues.UNIX,
+        //     [SEMATTRS_NET_TRANSPORT]: NetTransportValues.UNIX,
         //     ['net.sock.family']: 'unix',
         //   })
         }
@@ -73,7 +88,7 @@ export function httpTracer(inner: Deno.ServeHandler, opts?: {
         const resp = await inner(req, connInfo);
 
         serverSpan.addEvent('returned-http-response');
-        serverSpan.setAttribute(SemanticAttributes.HTTP_STATUS_CODE, resp.status);
+        serverSpan.setAttribute(SEMATTRS_HTTP_STATUS_CODE, resp.status);
         if (resp.statusText) {
           serverSpan.setAttribute('http.status_text', resp.statusText);
         }
@@ -87,10 +102,14 @@ export function httpTracer(inner: Deno.ServeHandler, opts?: {
 
         const respSnoop = snoopStream(resp.body);
         respSnoop.finalSize.then(size => {
-          serverSpan.setAttribute(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED, size);
+          serverSpan.setAttribute(SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED, size);
         }).catch(err => {
           // NOTE: err can be "resource closed" when the client walks away mid-response.
           serverSpan.recordException(err);
+          serverSpan.setStatus({
+            code: SpanStatusCode.ERROR,
+            message: `Response stream stopped before comppletion.`,
+          });
         }).finally(() => {
           inflightMetric.add(-1, reqMetricAttrs);
           serverSpan.end();
@@ -100,6 +119,10 @@ export function httpTracer(inner: Deno.ServeHandler, opts?: {
 
       } catch (err) {
         serverSpan.recordException(err);
+        serverSpan.setStatus({
+          code: SpanStatusCode.ERROR,
+          message: `Request handler rejected with ${err.name ?? err}`,
+        });
         serverSpan.end();
         inflightMetric.add(-1, reqMetricAttrs);
         throw err;

diff --git a/otel-platform/detectors.ts b/otel-platform/detectors.ts
@@ -1,9 +1,22 @@
 import { DetectorSync, Resource } from "../opentelemetry/resources.js";
-import { SemanticResourceAttributes } from "../opentelemetry/semantic-conventions.js";
+import {
+  SEMRESATTRS_CLOUD_PLATFORM,
+  SEMRESATTRS_CLOUD_PROVIDER,
+  SEMRESATTRS_CLOUD_REGION,
+  SEMRESATTRS_DEPLOYMENT_ENVIRONMENT,
+  SEMRESATTRS_FAAS_VERSION,
+  SEMRESATTRS_PROCESS_COMMAND,
+  SEMRESATTRS_PROCESS_COMMAND_ARGS,
+  SEMRESATTRS_PROCESS_EXECUTABLE_PATH,
+  SEMRESATTRS_PROCESS_PID,
+  SEMRESATTRS_PROCESS_RUNTIME_DESCRIPTION,
+  SEMRESATTRS_PROCESS_RUNTIME_NAME,
+  SEMRESATTRS_PROCESS_RUNTIME_VERSION,
+} from "../opentelemetry/semantic-conventions.js";
 
 const runtimeResource = new Resource({
-  [SemanticResourceAttributes.PROCESS_RUNTIME_NAME]: 'deno',
-  // [SemanticResourceAttributes.PROCESS_RUNTIME_DESCRIPTION]: 'Deno Runtime',
+  [SEMRESATTRS_PROCESS_RUNTIME_NAME]: 'deno',
+  // [SEMRESATTRS_PROCESS_RUNTIME_DESCRIPTION]: 'Deno Runtime',
 });
 export class DenoRuntimeDetector implements DetectorSync {
   detect() {
@@ -13,13 +26,14 @@ export class DenoRuntimeDetector implements DetectorSync {
     }
 
     // Deno Deploy does this:
-    if (!Deno.version?.deno) return new Resource({
-      [SemanticResourceAttributes.PROCESS_RUNTIME_NAME]: 'deno',
-      [SemanticResourceAttributes.PROCESS_RUNTIME_DESCRIPTION]: 'Deno Deploy hosted runtime',
-    });
+    if (!Deno.version?.deno) {
+      return runtimeResource.merge(new Resource({
+        [SEMRESATTRS_PROCESS_RUNTIME_DESCRIPTION]: 'Deno Deploy hosted runtime',
+      }));
+    }
 
     return runtimeResource.merge(new Resource({
-      [SemanticResourceAttributes.PROCESS_RUNTIME_VERSION]: Deno.version.deno,
+      [SEMRESATTRS_PROCESS_RUNTIME_VERSION]: Deno.version.deno,
     }));
   }
 }
@@ -42,18 +56,18 @@ export class DenoDeployDetector implements DetectorSync {
     }
 
     return new Resource({
-      [SemanticResourceAttributes.DEPLOYMENT_ENVIRONMENT]: 'production', // TODO: main branch or not?
-      [SemanticResourceAttributes.FAAS_VERSION]: deployVersion,
-      [SemanticResourceAttributes.CLOUD_REGION]: deployRegion,
-      [SemanticResourceAttributes.CLOUD_PROVIDER]: 'deno',
-      [SemanticResourceAttributes.CLOUD_PLATFORM]: 'deno_deploy',
+      [SEMRESATTRS_DEPLOYMENT_ENVIRONMENT]: 'production', // TODO: main branch or not?
+      [SEMRESATTRS_FAAS_VERSION]: deployVersion,
+      [SEMRESATTRS_CLOUD_REGION]: deployRegion,
+      [SEMRESATTRS_CLOUD_PROVIDER]: 'deno',
+      [SEMRESATTRS_CLOUD_PLATFORM]: 'deno_deploy',
     });
   }
 }
 
 const processResource = new Resource({
-  [SemanticResourceAttributes.PROCESS_PID]: Deno.pid,
-  [SemanticResourceAttributes.PROCESS_COMMAND_ARGS]: Deno.args,
+  [SEMRESATTRS_PROCESS_PID]: Deno.pid,
+  [SEMRESATTRS_PROCESS_COMMAND_ARGS]: Deno.args,
 });
 export class DenoProcessDetector implements DetectorSync {
   detect() {
@@ -62,8 +76,8 @@ export class DenoProcessDetector implements DetectorSync {
     if (!canRead) return processResource;
 
     return processResource.merge(new Resource({
-      [SemanticResourceAttributes.PROCESS_EXECUTABLE_PATH]: Deno.execPath(),
-      [SemanticResourceAttributes.PROCESS_COMMAND]: Deno.mainModule,
+      [SEMRESATTRS_PROCESS_EXECUTABLE_PATH]: Deno.execPath(),
+      [SEMRESATTRS_PROCESS_COMMAND]: Deno.mainModule,
     }));
   }
 }
diff --git a/sdk.ts b/sdk.ts
@@ -71,14 +71,7 @@ export class DenoTelemetrySdk {
     diag.setLogger(props?.diagLogger ?? new DiagConsoleLogger(), env.OTEL_LOG_LEVEL);
 
     this.resource = detectResourcesSync({
-      detectors: props?.detectors ?? [
-        new DenoRuntimeDetector(),
-        new DenoDeployDetector(),
-        new DenoProcessDetector(),
-        hostDetectorSync,
-        osDetectorSync,
-        envDetectorSync,
-      ],
+      detectors: props?.detectors ?? getDefaultDetectors(),
     });
     if (props?.resource) {
       this.resource = this.resource.merge(props.resource);
@@ -105,19 +98,18 @@ export class DenoTelemetrySdk {
     this.meter = new MeterProvider({
       resource: this.resource,
       views: props?.metricsViews,
+      // Metrics export on a fixed timer, so make the user opt-in to them
+      readers: ((props?.metricsExportIntervalMillis ?? 0) > 0) ? [
+        new PeriodicExportingMetricReader({
+          exporter: new OTLPMetricExporterBase(new OTLPMetricsExporter({
+            resourceBase: props?.otlpEndpointBase,
+          })),
+          exportIntervalMillis: props?.metricsExportIntervalMillis,
+        })
+      ] : [],
     });
     metrics.setGlobalMeterProvider(this.meter);
 
-    // Metrics export on a fixed timer, so make the user opt-in to them
-    if ((props?.metricsExportIntervalMillis ?? 0) > 0) {
-      this.meter.addMetricReader(new PeriodicExportingMetricReader({
-        exporter: new OTLPMetricExporterBase(new OTLPMetricsExporter({
-          resourceBase: props?.otlpEndpointBase,
-        })),
-        exportIntervalMillis: props?.metricsExportIntervalMillis,
-      }));
-    }
-
     this.logger = new LoggerProvider({
       resource: this.resource,
     });
@@ -131,7 +123,27 @@ export class DenoTelemetrySdk {
     registerInstrumentations({
       tracerProvider: this.tracer,
       meterProvider: this.meter,
+      loggerProvider: this.logger,
       instrumentations: props?.instrumentations ?? getDenoAutoInstrumentations(),
     });
   }
 }
+
+function getDefaultDetectors(): DetectorSync[] {
+  // We first check for Deno Deploy then decide what we want to detect based on that
+  const denoDeployDetector = new DenoDeployDetector();
+  const runtimeDetectors =
+    Object.keys(denoDeployDetector.detect().attributes).length
+      ? [denoDeployDetector]
+      : [
+          new DenoProcessDetector(),
+          hostDetectorSync,
+          osDetectorSync,
+        ];
+
+  return [
+    new DenoRuntimeDetector(),
+    ...runtimeDetectors,
+    envDetectorSync,
+  ];
+}