chore: urllib instrumentation; subprocess and similar

ops/_private/yaml.py

@@ -16,18 +16,23 @@
 
 from typing import Any, Optional, TextIO, Union
 
+import opentelemetry.trace
 import yaml
 
+tracer = opentelemetry.trace.get_tracer(__name__)
+
 # Use C speedups if available
 _safe_loader = getattr(yaml, 'CSafeLoader', yaml.SafeLoader)
 _safe_dumper = getattr(yaml, 'CSafeDumper', yaml.SafeDumper)
 
 
+@tracer.start_as_current_span('yaml.safe_load')  # type: ignore
 def safe_load(stream: Union[str, TextIO]) -> Any:
     """Same as yaml.safe_load, but use fast C loader if available."""
     return yaml.load(stream, Loader=_safe_loader)  # noqa: S506
 
 
+@tracer.start_as_current_span('yaml.safe_dump')  # type: ignore
 def safe_dump(data: Any, stream: Optional[TextIO] = None) -> str:
     """Same as yaml.safe_dump, but use fast C dumper if available."""
     return yaml.dump(data, stream=stream, Dumper=_safe_dumper)  # type: ignore

ops/log.py

@@ -14,6 +14,7 @@
 
 """Interface to emit messages to the Juju logging system."""
 
+import contextvars
 import logging
 import sys
 import types
@@ -25,17 +26,34 @@
 class JujuLogHandler(logging.Handler):
     """A handler for sending logs to Juju via juju-log."""
 
+    foo: contextvars.ContextVar[bool]
+
     def __init__(self, model_backend: _ModelBackend, level: int = logging.DEBUG):
         super().__init__(level)
         self.model_backend = model_backend
+        self.foo = contextvars.ContextVar('foo', default=False)
 
     def emit(self, record: logging.LogRecord):
         """Send the specified logging record to the Juju backend.
 
         This method is not used directly by the ops library, but by
         :class:`logging.Handler` itself as part of the logging machinery.
         """
-        self.model_backend.juju_log(record.levelname, self.format(record))
+        # FIXME strategies to prevent infinite recursion
+        # 1. block OTEL logging (as well as any other logging, really)
+        # until this function is done.
+        # which is a bit tricky when we've got a helper OTEL thread.
+        # 2. pre-emptively disable or filter out logs from OTEL
+        # 3. prevent recursion on this very method only
+        # ^--- will start with this, pending team discussion
+        if self.foo.get():
+            return
+
+        token = self.foo.set(True)
+        try:
+            self.model_backend.juju_log(record.levelname, self.format(record))
+        finally:
+            self.foo.reset(token)
 
 
 def setup_root_logging(

ops/model.py

@@ -57,6 +57,8 @@
     get_args,
 )
 
+import opentelemetry.trace
+
 import ops
 import ops.pebble as pebble
 from ops._private import timeconv, yaml
@@ -111,6 +113,7 @@
 
 
 logger = logging.getLogger(__name__)
+tracer = opentelemetry.trace.get_tracer(__name__)
 
 MAX_LOG_LINE_LEN = 131071  # Max length of strings to pass to subshell.
 
@@ -2748,7 +2751,9 @@ def pull_path(
                     dstpath.parent.mkdir(parents=True, exist_ok=True)
                     with self.pull(info.path, encoding=None) as src:
                         with dstpath.open(mode='wb') as dst:
-                            shutil.copyfileobj(src, dst)
+                            with tracer.start_as_current_span('shutil.copyfileobj') as span:  # type: ignore
+                                span.set_attribute('dstpath', str(dstpath))  # type: ignore
+                                shutil.copyfileobj(src, dst)
             except (OSError, pebble.Error) as err:
                 errors.append((str(source_path), err))
         if errors:
@@ -3308,9 +3313,11 @@ def _run(
         }
         if input_stream:
             kwargs.update({'input': input_stream})
-        which_cmd = shutil.which(args[0])
-        if which_cmd is None:
-            raise RuntimeError(f'command not found: {args[0]}')
+        with tracer.start_as_current_span('shutil.which') as span:  # type: ignore
+            span.set_attribute('program name', args[0])  # type: ignore
+            which_cmd = shutil.which(args[0])
+            if which_cmd is None:
+                raise RuntimeError(f'command not found: {args[0]}')
         args = (which_cmd,) + args[1:]
         if use_json:
             args += ('--format=json',)
@@ -3471,7 +3478,9 @@ def pod_spec_set(
                 args.extend(['--k8s-resources', str(k8s_res_path)])
             self._run('pod-spec-set', *args)
         finally:
-            shutil.rmtree(str(tmpdir))
+            with tracer.start_as_current_span('shutil.rmtree') as span:  # type: ignore
+                span.set_attribute('tmpdir', str(tmpdir))  # type: ignore
+                shutil.rmtree(str(tmpdir))
 
     def status_get(self, *, is_app: bool = False) -> '_StatusDict':
         """Get a status of a unit or an application.

ops/pebble.py

@@ -79,6 +79,7 @@
     Union,
 )
 
+import opentelemetry.trace
 import websocket
 
 from ops._private import timeconv, yaml
@@ -343,6 +344,7 @@ def recv(self) -> Union[str, bytes]: ...
 
 
 logger = logging.getLogger(__name__)
+tracer = opentelemetry.trace.get_tracer(__name__)
 
 
 class _NotProvidedFlag:
@@ -2028,6 +2030,7 @@ def _ensure_content_type(
             raise ProtocolError(f'expected Content-Type {expected!r}, got {ctype!r}')
         return options
 
+    @tracer.start_as_current_span('Pebble._request_raw')  # type: ignore
     def _request_raw(
         self,
         method: str,

ops/tracing/_fixme.py

@@ -23,6 +23,7 @@
     encode_spans,
 )
 from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter
+from opentelemetry.instrumentation.urllib import URLLibInstrumentor  # type: ignore
 from opentelemetry.sdk.resources import Resource
 from opentelemetry.sdk.trace import ReadableSpan, TracerProvider
 from opentelemetry.sdk.trace.export import BatchSpanProcessor, SpanExporter, SpanExportResult
@@ -33,6 +34,8 @@
 import ops.tracing._buffer
 
 logger = logging.getLogger(__name__)
+# Trace `urllib` usage when talking to Pebble
+URLLibInstrumentor().instrument()
 
 _OTLP_SPAN_EXPORTER_TIMEOUT = 1  # seconds
 """How much to give OTLP span exporter has to push traces to the backend."""
@@ -57,6 +60,7 @@ def export(self, spans: Sequence[ReadableSpan]) -> SpanExportResult:
         # - 2s for live data
         deadline = time.monotonic() + 6
 
+        # FIXME remove this dev crud
         # __import__("pdb").set_trace()
         import threading
 

pyproject.toml

@@ -43,13 +43,8 @@ testing = [
     "ops-scenario>=7.0.5,<8",
 ]
 tracing = [
-    "opentelemetry-exporter-otlp-proto-http~=1.21",  # TODO: copied from the charm lib, no idea about the version range
-    # TODO: the above already brings in -api, -sdk, etc.
-    # Should we be more explicit?
-    # E.g. require packages that provide APIs we use explicitly,
-    # as in, could the exporter suddenly no longer require -sdk and
-    # we'd be left without the transient dep we need?
-    # https://github.com/open-telemetry/opentelemetry-python/blob/032784d3c230312e8fa74d95a41a0253a719a843/exporter/opentelemetry-exporter-otlp-proto-http/pyproject.toml#L29-L37
+    "opentelemetry-exporter-otlp-proto-http~=1.29",  # TODO: copied from the charm lib, no idea about the version range
+    "opentelemetry-instrumentation-urllib~=0.50b0",
 ]
 # Empty for now, because Harness is bundled with the base install, but allow
 # specifying the extra to ease transition later.