sdk-telemetry: tighten cross-repo counter contract + auto atexit + scheme handling

src/layerlens/_telemetry.py

@@ -33,6 +33,7 @@
 """
 from __future__ import annotations
 
+import atexit
 import contextlib
 import os
 import time
@@ -43,6 +44,7 @@
 _meter: Any = None
 _counter_events: Any = None
 _hist_request_duration: Any = None
+_atexit_registered: bool = False
 
 
 def _enabled() -> bool:
@@ -85,9 +87,23 @@ def _try_init() -> bool:
     endpoint = os.environ.get(
         "LAYERLENS_OTLP_ENDPOINT", "https://otel.layerlens.ai:4317"
     )
-    insecure = (
-        os.environ.get("LAYERLENS_OTLP_INSECURE", "false").lower() == "true"
-    )
+
+    # Resolve insecure-vs-TLS from the endpoint scheme when one is present,
+    # falling back to the explicit env flag for schemeless endpoints (the
+    # OTel gRPC convention of "host:port"). This avoids the ambiguity where
+    # a caller sets LAYERLENS_OTLP_ENDPOINT=https://... AND
+    # LAYERLENS_OTLP_INSECURE=true — previously the flags would disagree,
+    # and the OTLP gRPC exporter's behaviour was version-dependent. Now
+    # the scheme, when present, is authoritative.
+    insecure_env = os.environ.get("LAYERLENS_OTLP_INSECURE", "").strip().lower()
+    if endpoint.startswith("https://"):
+        insecure = False
+        endpoint = endpoint[len("https://") :]
+    elif endpoint.startswith("http://"):
+        insecure = True
+        endpoint = endpoint[len("http://") :]
+    else:
+        insecure = insecure_env == "true"
 
     resource = Resource.create({
         "service.name": "atlas-sdk-python",
@@ -122,6 +138,18 @@ def _try_init() -> bool:
         _hist_request_duration = None
         return False
 
+    # Auto-register atexit shutdown on first successful init. Without this,
+    # SDK consumers that don't manually call _telemetry.shutdown() would
+    # lose up to 30 s of buffered events on clean process exit (the
+    # PeriodicExportingMetricReader flush interval). The CLI also calls
+    # atexit.register(_telemetry.shutdown) explicitly — the registered-once
+    # guard here means a second registration is a no-op, keeping shutdown
+    # idempotent regardless of call path.
+    global _atexit_registered
+    if not _atexit_registered:
+        atexit.register(shutdown)
+        _atexit_registered = True
+
     return True
 
 
@@ -134,16 +162,24 @@ def event(
     """Increment ``atlas_sdk_events_total{surface, event}`` by 1.
 
     No-op when telemetry is disabled or OTel SDK is absent.
+
+    The ``attributes`` parameter is accepted for forward compatibility and
+    (when present) is attached to the **OTel span** covering the timed()
+    context — NOT to the counter. The atlas-app Prometheus counter
+    ``atlas_sdk_events_total`` is declared with exactly 2 labels
+    (``surface, event``) in ``apps/shared/observability/metrics.go``; any
+    third attribute on the counter would create a divergent label set that
+    the server-side declaration and dashboards don't anticipate. Extra
+    attributes are therefore dropped here by design — keep atlas-app as
+    the single source of truth for the counter's label schema.
     """
     if not _try_init() or _counter_events is None:
         return
+    # Exactly two labels — MUST match the atlas-app Go declaration.
     attrs: dict[str, Any] = {"surface": surface, "event": event_name}
-    if attributes:
-        # Only allow a small allowlist of attribute keys — no PII.
-        allow = {"command", "resource", "outcome", "status_code"}
-        for k, v in attributes.items():
-            if k in allow and isinstance(v, (str, int, bool, float)):
-                attrs[k] = str(v)
+    # Note: `attributes` is intentionally ignored for the counter. See
+    # docstring above for rationale + where richer dimensions belong.
+    _ = attributes
     try:
         _counter_events.add(1, attributes=attrs)
     except Exception:

tests/test_telemetry.py

@@ -61,12 +61,24 @@ def test_event_with_telemetry_off_doesnt_import_otel(monkeypatch, _reset_telemet
     assert "opentelemetry" not in sys.modules
 
 
-def test_attributes_allowlist(monkeypatch, _reset_telemetry_module):
-    """Disallowed attribute keys are silently dropped, not raised."""
+def test_counter_labels_match_atlas_app_declaration(
+    monkeypatch, _reset_telemetry_module
+):
+    """The Prometheus counter atlas_sdk_events_total is declared with EXACTLY
+    two labels on the atlas-app server side (surface, event). To keep the
+    two-repo contract aligned, the SDK MUST NOT add any third attribute to
+    the counter — regardless of what the caller passes via ``attributes=``.
+    Any extra attribute WOULD create a divergent label set that server-side
+    dashboards and recording rules don't anticipate.
+
+    This test asserts that even attributes a previous allowlist permitted
+    (command, outcome, etc.) are now dropped. Extra dimensions belong on
+    OTel span attributes, not on the counter — atlas-app is the single
+    source of truth for the counter's label schema.
+    """
     t = _reset_telemetry_module
     monkeypatch.setenv("LAYERLENS_TELEMETRY", "on")
 
-    # Stub OTel SDK so we can observe what reaches add().
     seen_attrs: dict = {}
 
     class _StubCounter:
@@ -82,19 +94,17 @@ def add(self, value, attributes=None):
         "cli",
         "cmd_run",
         attributes={
-            "command": "trace ls",
-            "email": "user@example.com",  # MUST be dropped
-            "ip": "10.0.0.1",  # MUST be dropped
-            "outcome": "success",
+            "command": "trace ls",  # dropped by the 2-label contract
+            "email": "user@example.com",  # dropped (PII)
+            "ip": "10.0.0.1",  # dropped (PII)
+            "outcome": "success",  # dropped by the 2-label contract
         },
     )
 
-    assert seen_attrs.get("surface") == "cli"
-    assert seen_attrs.get("event") == "cmd_run"
-    assert seen_attrs.get("command") == "trace ls"
-    assert seen_attrs.get("outcome") == "success"
-    assert "email" not in seen_attrs
-    assert "ip" not in seen_attrs
+    # Exactly surface + event, nothing more.
+    assert set(seen_attrs.keys()) == {"surface", "event"}
+    assert seen_attrs["surface"] == "cli"
+    assert seen_attrs["event"] == "cmd_run"
 
 
 def test_event_swallows_counter_errors(monkeypatch, _reset_telemetry_module):