Warn if service name is not set (#567)

* Add support for env vars in spec: * SPLUNK_PROFILER_LOGS_ENDPOINT * SPLUNK_PROFILER_CALL_STACK_INTERVAL * Lint * Add and use env getint for profiling interval * Lint * Warn if service name is not set. Previously only warned if profiling was enabled. * Add oteltest for missing svcname * Use constant for service name
signalfx · Dec 6, 2024 · 5dfb3b5 · 5dfb3b5
1 parent 190828d
commit 5dfb3b5
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 24 deletions.
diff --git a/src/splunk_otel/distro.py b/src/splunk_otel/distro.py
@@ -20,6 +20,7 @@
     OTEL_EXPORTER_OTLP_HEADERS,
     OTEL_EXPORTER_OTLP_LOGS_ENDPOINT,
     OTEL_RESOURCE_ATTRIBUTES,
+    OTEL_SERVICE_NAME,
 )
 
 from splunk_otel.__about__ import __version__ as version
@@ -35,7 +36,14 @@
 )
 from splunk_otel.propagator import ServerTimingResponsePropagator
 
-DISTRO_NAME = "splunk-opentelemetry"
+_DISTRO_NAME = "splunk-opentelemetry"
+
+_NO_SERVICE_NAME_WARNING = """The service.name attribute is not set, which may make your service difficult to identify.
+Set your service name using the OTEL_SERVICE_NAME environment variable.
+e.g. `OTEL_SERVICE_NAME="<YOUR_SERVICE_NAME_HERE>"`"""
+_DEFAULT_SERVICE_NAME = "unnamed-python-service"
+
+_pylogger = logging.getLogger(__name__)
 
 
 class SplunkDistro(BaseDistro):
@@ -50,11 +58,17 @@ def __init__(self):
 
     def _configure(self, **kwargs):
         self.set_env_defaults()
+        self.check_service_name()
         self.set_profiling_env()
         self.set_resource_attributes()
         self.configure_headers()
         self.set_server_timing_propagator()
 
+    def check_service_name(self):
+        if not len(self.env.getval(OTEL_SERVICE_NAME)):
+            _pylogger.warning(_NO_SERVICE_NAME_WARNING)
+            self.env.setval(OTEL_SERVICE_NAME, _DEFAULT_SERVICE_NAME)
+
     def set_env_defaults(self):
         for key, value in DEFAULTS.items():
             self.env.setdefault(key, value)
@@ -67,7 +81,7 @@ def set_profiling_env(self):
                 self.env.setval(OTEL_EXPORTER_OTLP_LOGS_ENDPOINT, logs_endpt)
 
     def set_resource_attributes(self):
-        self.env.list_append(OTEL_RESOURCE_ATTRIBUTES, f"telemetry.distro.name={DISTRO_NAME}")
+        self.env.list_append(OTEL_RESOURCE_ATTRIBUTES, f"telemetry.distro.name={_DISTRO_NAME}")
         self.env.list_append(OTEL_RESOURCE_ATTRIBUTES, f"telemetry.distro.version={version}")
 
     def configure_headers(self):

diff --git a/src/splunk_otel/profile.py b/src/splunk_otel/profile.py
@@ -22,14 +22,10 @@
 from splunk_otel import profile_pb2
 from splunk_otel.env import SPLUNK_PROFILER_CALL_STACK_INTERVAL, SPLUNK_PROFILER_ENABLED, Env
 
-DEFAULT_PROF_CALL_STACK_INTERVAL_MILLIS = 1000
+_DEFAULT_PROF_CALL_STACK_INTERVAL_MILLIS = 1000
 
 _SERVICE_NAME_ATTR = "service.name"
 _SPLUNK_DISTRO_VERSION_ATTR = "splunk.distro.version"
-_NO_SERVICE_NAME_WARNING = """The service.name attribute is not set, which may make your service difficult to identify.
-Set your service name using the OTEL_SERVICE_NAME environment variable.
-e.g. `OTEL_SERVICE_NAME="<YOUR_SERVICE_NAME_HERE>"`"""
-_DEFAULT_SERVICE_NAME = "unnamed-python-service"
 
 _profile_timer = None
 _pylogger = logging.getLogger(__name__)
@@ -43,7 +39,7 @@ def _start_profiling_if_enabled(env=None):
 
 def start_profiling(env=None):
     env = env or Env()
-    interval_millis = env.getint(SPLUNK_PROFILER_CALL_STACK_INTERVAL, DEFAULT_PROF_CALL_STACK_INTERVAL_MILLIS)
+    interval_millis = env.getint(SPLUNK_PROFILER_CALL_STACK_INTERVAL, _DEFAULT_PROF_CALL_STACK_INTERVAL_MILLIS)
     svcname = env.getval(OTEL_SERVICE_NAME)
 
     tcm = _ThreadContextMapping()
@@ -63,15 +59,10 @@ def stop_profiling():
 
 
 def _mk_resource(service_name) -> Resource:
-    if service_name:
-        resolved_name = service_name
-    else:
-        _pylogger.warning(_NO_SERVICE_NAME_WARNING)
-        resolved_name = _DEFAULT_SERVICE_NAME
     return Resource.create(
         {
             _SPLUNK_DISTRO_VERSION_ATTR: version,
-            _SERVICE_NAME_ATTR: resolved_name,
+            _SERVICE_NAME_ATTR: service_name,
         }
     )
 

diff --git a/tests/ott_spec.py b/tests/ott_spec.py
@@ -1,16 +1,20 @@
-from oteltest.telemetry import extract_leaves, get_attribute
 from ott_lib import project_path, trace_loop
 
+SERVICE_NAME = "spec-svc"
+
 if __name__ == "__main__":
     trace_loop(1)
 
 
 class SpecOtelTest:
     def requirements(self):
-        return project_path(), "oteltest"
+        return (project_path(),)
 
     def environment_variables(self):
-        return {"OTEL_SERVICE_NAME": "my-svc"}
+        return {
+            "OTEL_PYTHON_DISABLED_INSTRUMENTATIONS": "system_metrics",
+            "OTEL_SERVICE_NAME": SERVICE_NAME,
+        }
 
     def wrapper_command(self):
         return "opentelemetry-instrument"
@@ -19,20 +23,27 @@ def on_start(self):
         return None
 
     def on_stop(self, telemetry, stdout: str, stderr: str, returncode: int) -> None:
-        attributes = extract_leaves(telemetry, "trace_requests", "pbreq", "resource_spans", "resource", "attributes")
+        from oteltest.telemetry import extract_leaves, get_attribute
+
+        attributes = extract_leaves(
+            telemetry,
+            "trace_requests",
+            "pbreq",
+            "resource_spans",
+            "resource",
+            "attributes",
+        )
 
         assert get_attribute(attributes, "telemetry.sdk.name")
         assert get_attribute(attributes, "telemetry.sdk.version")
         assert get_attribute(attributes, "telemetry.sdk.language")
 
-        assert get_attribute_str(attributes, "telemetry.distro.version")
-        assert get_attribute_str(attributes, "telemetry.distro.name") == "splunk-opentelemetry"
+        assert get_attribute(attributes, "telemetry.distro.version").value.string_value
+        assert get_attribute(attributes, "telemetry.distro.name").value.string_value == "splunk-opentelemetry"
 
         assert get_attribute(attributes, "process.pid")
 
+        assert get_attribute(attributes, "service.name").value.string_value == SERVICE_NAME
+
     def is_http(self):
         return False
-
-
-def get_attribute_str(attributes, key):
-    return get_attribute(attributes, key).value.string_value
diff --git a/tests/ott_svcname_unset.py b/tests/ott_svcname_unset.py
@@ -0,0 +1,38 @@
+from ott_lib import project_path, trace_loop
+
+if __name__ == "__main__":
+    trace_loop(1)
+
+
+class SpecOtelTest:
+    def requirements(self):
+        return (project_path(),)
+
+    def environment_variables(self):
+        return {
+            "OTEL_PYTHON_DISABLED_INSTRUMENTATIONS": "system_metrics",
+        }
+
+    def wrapper_command(self):
+        return "opentelemetry-instrument"
+
+    def on_start(self):
+        return None
+
+    def on_stop(self, telemetry, stdout: str, stderr: str, returncode: int) -> None:
+        from oteltest.telemetry import extract_leaves, get_attribute
+
+        assert "service.name attribute is not set" in stderr
+
+        attributes = extract_leaves(
+            telemetry,
+            "trace_requests",
+            "pbreq",
+            "resource_spans",
+            "resource",
+            "attributes",
+        )
+        assert get_attribute(attributes, "service.name").value.string_value == "unnamed-python-service"
+
+    def is_http(self):
+        return False
diff --git a/tests/test_distro.py b/tests/test_distro.py
@@ -11,6 +11,8 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
+import logging
+
 from opentelemetry.instrumentation.propagators import (
     get_global_response_propagator,
     set_global_response_propagator,
@@ -110,6 +112,14 @@ def test_resource_attributes():
     assert "foo=bar" in attrs
 
 
+def test_service_name(caplog):
+    with caplog.at_level(logging.WARNING):
+        env_store = {}
+        configure_distro(env_store)
+        assert "OTEL_SERVICE_NAME" in env_store
+    assert "service.name attribute is not set" in caplog.text
+
+
 def configure_distro(env_store):
     sd = SplunkDistro()
     sd.env = Env(env_store)