From 510175427365b7d9375ecf964d0a08abba8af9ff Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Thu, 19 Sep 2024 15:53:59 +0800 Subject: [PATCH] feedback Signed-off-by: Adrian Cole --- .otel.env | 5 ----- CONTRIBUTING.md | 5 ++--- justfile | 2 +- src/goose/cli/session.py | 25 ++++++++++++++++--------- 4 files changed, 19 insertions(+), 18 deletions(-) delete mode 100644 .otel.env diff --git a/.otel.env b/.otel.env deleted file mode 100644 index 8470429e5..000000000 --- a/.otel.env +++ /dev/null @@ -1,5 +0,0 @@ -OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 -OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf -OTEL_SERVICE_NAME=goose -OTEL_METRIC_EXPORT_INTERVAL=100 -OTEL_BSP_SCHEDULE_DELAY=100 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a2770aab9..b4d1f2520 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -116,13 +116,12 @@ of the underlying HTTP requests. For example, a 404 could be indicative of an in First, ensure you have an OpenTelemetry compatible collector listening on port 4318, such as [otel-tui][otel-tui]. ```bash -brew install ymtdzzz/tap/otel-tui -otel-tui +docker run --rm -it --name otel-tui -p 4318:4318 ymtdzzz/otel-tui ``` Then, start goose like this: ```bash -uv run dotenv -f ./.otel.env run -- opentelemetry-instrument goose session start +uv run opentelemetry-instrument --service_name goose --exporter_otlp_protocol http/protobuf goose session start # or `just otel-goose session start` ``` diff --git a/justfile b/justfile index ce69ea6c6..889c39f84 100644 --- a/justfile +++ b/justfile @@ -22,4 +22,4 @@ docs: cd docs && uv sync && uv run mkdocs serve otel-goose *FLAGS: - uv run dotenv -f ./.otel.env run -- opentelemetry-instrument goose {{FLAGS}} + uv run opentelemetry-instrument --service_name goose --exporter_otlp_protocol http/protobuf goose {{FLAGS}} diff --git a/src/goose/cli/session.py b/src/goose/cli/session.py index 28635540c..c1875efd3 100644 --- a/src/goose/cli/session.py +++ b/src/goose/cli/session.py @@ -92,12 +92,15 @@ def __init__( name: Optional[str] = None, profile: Optional[str] = None, plan: Optional[dict] = None, - tracer: trace.Tracer = trace.get_tracer(__name__), + tracer: trace.Tracer = trace.get_tracer("goose"), **kwargs: Dict[str, Any], ) -> None: self.name = name self.status_indicator = Status("", spinner="dots") self.notifier = SessionNotifier(self.status_indicator) + + # Set the tracer as a field in session, as opposed to a module variable + # so that tests can swap this out and safely run in parallel. self.tracer = tracer self.exchange = build_exchange(profile=load_profile(profile), notifier=self.notifier) @@ -154,20 +157,24 @@ def run(self) -> None: """ message = self.process_first_message() while message: # Loop until no input (empty string). - # Start a span with the default tracer. This is present with opentelemetry-instrument - with self.tracer.start_as_current_span(message.role) as span: - span.set_attribute("goose.role", message.role) - span.set_attribute("goose.id", message.id) - # For starters, only add to the trace the first text content - first_content = message.content[0] if message.content else None - if isinstance(first_content, Text): - span.set_attribute("goose.text", first_content.text) + span_attributes = { + "goose.role": message.role, + "goose.id": message.id, + } + + # For starters, only add to the trace the first text content + first_content = message.content[0] if message.content else None + if isinstance(first_content, Text): + span_attributes["goose.text"] = first_content.text + with self.tracer.start_as_current_span(message.role, attributes=span_attributes) as span: self.notifier.start() try: self.exchange.add(message) self.reply() # Process the user message. except KeyboardInterrupt: + # TODO: should we make interrupting an error? If not, how should we mark this? + # span as it was interrupted? span.set_status(OtelStatus(OtelStatusCode.ERROR, "KeyboardInterrupt")) self.interrupt_reply() except Exception as e: