From a8af4405cb186620cfd1cc309973208d5773ba3a Mon Sep 17 00:00:00 2001
From: Bastian Krol <bastian.krol@dash0.com>
Date: Tue, 17 Sep 2024 11:22:59 +0200
Subject: [PATCH] fix: when auto-detecting service name, use version as
 service.version

(instead of concatenating it into service.name)
---
 .../index.ts                                        | 11 +++++++----
 .../index_test.ts                                   | 13 ++++++++-----
 test/integration/test.ts                            |  4 ++--
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/src/detectors/node/opentelemetry-resource-detector-service-name-fallback/index.ts b/src/detectors/node/opentelemetry-resource-detector-service-name-fallback/index.ts
index f2ddca0..b51e0f5 100644
--- a/src/detectors/node/opentelemetry-resource-detector-service-name-fallback/index.ts
+++ b/src/detectors/node/opentelemetry-resource-detector-service-name-fallback/index.ts
@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { DetectorSync, Resource } from '@opentelemetry/resources';
-import { SEMRESATTRS_SERVICE_NAME } from '@opentelemetry/semantic-conventions';
+import { SEMRESATTRS_SERVICE_NAME, SEMRESATTRS_SERVICE_VERSION } from '@opentelemetry/semantic-conventions';
 
 import { readPackageJson } from './packageJsonUtil';
 
@@ -15,7 +15,7 @@ export default class ServiceNameFallbackDetector implements DetectorSync {
     if (
       hasOptedOutOfServiceNameFallbackDetection() ||
       hasOTelServiceNameSet() ||
-      hasServiceNameSetViaResourceAttributesThing()
+      hasServiceNameSetViaOTelResourceAttributesEnvVar()
     ) {
       return {};
     }
@@ -24,7 +24,10 @@ export default class ServiceNameFallbackDetector implements DetectorSync {
     if (!packageJson) {
       return {};
     }
-    return { [SEMRESATTRS_SERVICE_NAME]: `${packageJson.name}@${packageJson.version}` };
+    return {
+      [SEMRESATTRS_SERVICE_NAME]: packageJson.name,
+      [SEMRESATTRS_SERVICE_VERSION]: packageJson.version,
+    };
   }
 }
 
@@ -38,7 +41,7 @@ function hasOTelServiceNameSet() {
   return otelServiceName && otelServiceName.trim() !== '';
 }
 
-function hasServiceNameSetViaResourceAttributesThing() {
+function hasServiceNameSetViaOTelResourceAttributesEnvVar() {
   const otelResourceAttributes = process.env.OTEL_RESOURCE_ATTRIBUTES;
   if (otelResourceAttributes) {
     const rawAttributes: string[] = otelResourceAttributes.split(',');
diff --git a/src/detectors/node/opentelemetry-resource-detector-service-name-fallback/index_test.ts b/src/detectors/node/opentelemetry-resource-detector-service-name-fallback/index_test.ts
index 6f33b1e..5edd05e 100644
--- a/src/detectors/node/opentelemetry-resource-detector-service-name-fallback/index_test.ts
+++ b/src/detectors/node/opentelemetry-resource-detector-service-name-fallback/index_test.ts
@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { Resource } from '@opentelemetry/resources';
-import { SEMRESATTRS_SERVICE_NAME } from '@opentelemetry/semantic-conventions';
+import { SEMRESATTRS_SERVICE_NAME, SEMRESATTRS_SERVICE_VERSION } from '@opentelemetry/semantic-conventions';
 import { expect } from 'chai';
 import Sinon from 'sinon';
 import sinon from 'sinon';
@@ -57,11 +57,12 @@ describe('service name fallback', () => {
     });
   });
 
-  it('sets a service name based on package.json attributes', async () => {
+  it('sets a service name and version based on package.json attributes', async () => {
     givenAValidPackageJsonFile();
     const result = serviceNameFallback.detect();
     const attributes = await waitForAsyncDetection(result);
-    expect(attributes).to.have.property(SEMRESATTRS_SERVICE_NAME, '@example/app-under-test@2.13.47');
+    expect(attributes).to.have.property(SEMRESATTRS_SERVICE_NAME, '@example/app-under-test');
+    expect(attributes).to.have.property(SEMRESATTRS_SERVICE_VERSION, '2.13.47');
   });
 
   it('does not set a service name if DASH0_AUTOMATIC_SERVICE_NAME is false', async () => {
@@ -85,7 +86,8 @@ describe('service name fallback', () => {
     process.env.OTEL_SERVICE_NAME = '   ';
     const result = serviceNameFallback.detect();
     const attributes = await waitForAsyncDetection(result);
-    expect(attributes).to.have.property(SEMRESATTRS_SERVICE_NAME, '@example/app-under-test@2.13.47');
+    expect(attributes).to.have.property(SEMRESATTRS_SERVICE_NAME, '@example/app-under-test');
+    expect(attributes).to.have.property(SEMRESATTRS_SERVICE_VERSION, '2.13.47');
   });
 
   it('does not set a service name if OTEL_RESOURCE_ATTRIBUTES has the service.name key', async () => {
@@ -101,7 +103,8 @@ describe('service name fallback', () => {
     process.env.OTEL_RESOURCE_ATTRIBUTES = 'key1=value,key2=value';
     const result = serviceNameFallback.detect();
     const attributes = await waitForAsyncDetection(result);
-    expect(attributes).to.have.property(SEMRESATTRS_SERVICE_NAME, '@example/app-under-test@2.13.47');
+    expect(attributes).to.have.property(SEMRESATTRS_SERVICE_NAME, '@example/app-under-test');
+    expect(attributes).to.have.property(SEMRESATTRS_SERVICE_VERSION, '2.13.47');
   });
 
   it('does not set a service name if no package.json can be found', async () => {
diff --git a/test/integration/test.ts b/test/integration/test.ts
index f1bfc5a..e0f0c88 100644
--- a/test/integration/test.ts
+++ b/test/integration/test.ts
@@ -177,8 +177,8 @@ describe('attach', () => {
         expectMatchingSpan(
           traces,
           [
-            resource =>
-              expectResourceAttribute(resource, 'service.name', 'dash0-app-under-test-express-typescript@1.0.0'),
+            resource => expectResourceAttribute(resource, 'service.name', 'dash0-app-under-test-express-typescript'),
+            resource => expectResourceAttribute(resource, 'service.version', '1.0.0'),
           ],
           [
             span => expect(span.kind).to.equal(SpanKind.SERVER, 'span kind should be server'),