From ef64fedbd929239bbca7db7b18c6a6ebe637b8b6 Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Mon, 20 Mar 2023 20:49:52 -0400 Subject: [PATCH 1/2] Only enable OTLP exporter when configured This wasn't a problem in the past, since we only enabled automatic instrumentation when this was set up, but it does cause errors when trying to add manual instrumentation. --- bookwyrm/telemetry/open_telemetry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/telemetry/open_telemetry.py b/bookwyrm/telemetry/open_telemetry.py index aff68a8b3..e405011d9 100644 --- a/bookwyrm/telemetry/open_telemetry.py +++ b/bookwyrm/telemetry/open_telemetry.py @@ -10,7 +10,7 @@ if settings.OTEL_EXPORTER_CONSOLE: trace.get_tracer_provider().add_span_processor( BatchSpanProcessor(ConsoleSpanExporter()) ) -else: +elif settings.OTEL_EXPORTER_OTLP_ENDPOINT: trace.get_tracer_provider().add_span_processor( BatchSpanProcessor(OTLPSpanExporter()) ) From 7efbdb186569c537f8e7b58a04ed8074862e0e62 Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Mon, 20 Mar 2023 15:49:09 -0400 Subject: [PATCH 2/2] Add more detailed telemetry for get_audience This is still slow in some cases, despite #2723, so this information should give useful data about how it could be optimized more. This also adds some abstraction around getting the tracer, just to follow the advice in the OpenTelemetry documentation not to use __name__ directly to set the tracer name. The advice is ignored in most of their examples, so it probably doesn't matter, but IDK, seems reasonable to try to follow it. Related: #2720 --- bookwyrm/activitystreams.py | 11 ++++++++++- bookwyrm/telemetry/open_telemetry.py | 4 ++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index d4dac1412..74471883e 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -4,10 +4,15 @@ from django.dispatch import receiver from django.db import transaction from django.db.models import signals, Q from django.utils import timezone +from opentelemetry import trace from bookwyrm import models from bookwyrm.redis_store import RedisStore, r from bookwyrm.tasks import app, LOW, MEDIUM, HIGH +from bookwyrm.telemetry import open_telemetry + + +tracer = open_telemetry.tracer() class ActivityStream(RedisStore): @@ -136,8 +141,10 @@ class ActivityStream(RedisStore): ) return audience.distinct() - def get_audience(self, status): # pylint: disable=no-self-use + @tracer.start_as_current_span("ActivityStream.get_audience") + def get_audience(self, status): """given a status, what users should see it""" + trace.get_current_span().set_attribute("stream_id", self.key) return [user.id for user in self._get_audience(status)] def get_stores_for_object(self, obj): @@ -160,7 +167,9 @@ class HomeStream(ActivityStream): key = "home" + @tracer.start_as_current_span("HomeStream.get_audience") def get_audience(self, status): + trace.get_current_span().set_attribute("stream_id", self.key) audience = super()._get_audience(status) if not audience: return [] diff --git a/bookwyrm/telemetry/open_telemetry.py b/bookwyrm/telemetry/open_telemetry.py index e405011d9..2798582d0 100644 --- a/bookwyrm/telemetry/open_telemetry.py +++ b/bookwyrm/telemetry/open_telemetry.py @@ -29,3 +29,7 @@ def instrumentCelery(): @worker_process_init.connect(weak=False) def init_celery_tracing(*args, **kwargs): CeleryInstrumentor().instrument() + + +def tracer(): + return trace.get_tracer(__name__)