Skip to content

fix(tracing): fix tracer provider initialization logic #381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kevinlin09 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(tracing): fix tracer provider initialization logic #381

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 30 additions & 21 deletions src/agentscope_runtime/engine/tracing/wrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,29 +935,38 @@ def _has_existing_trace_provider() -> bool:
return True

def _get_ot_tracer_inner() -> ot_trace.Tracer:
if _has_existing_trace_provider():
enable_report = _str_to_bool(os.getenv("TRACE_ENABLE_REPORT", "false"))
enable_debug = _str_to_bool(os.getenv("TRACE_ENABLE_DEBUG", "false"))

if enable_report and _has_existing_trace_provider():

@pitt-liang pitt-liang Jan 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the use case for this?

If the user has already configured a trace_provider at the beginning, do they still need to additionally set TRACE_ENABLE_REPORT in order to reuse the existing trace_provider?

If so, that seems rather odd. The user has already configured the trace_provider according to the OpenTelemetry specification (In our cases), but now they also need to learn or inspect the code to discover that there is an additional TRACE_ENABLE_REPORT environment variable.

@pitt-liang pitt-liang Jan 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main point is this: if the user has already configured a global trace_provider in their code (and has likely also set up the corresponding exporter), shouldn't the system follow that configuration by default? It seems unnecessar to require the user to additionally set the TRACE_ENABLE_REPORT environment variable in order to activate logic that leverages an already-configured trace_provider.

Copilot AI Jan 14, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition checking enable_report and _has_existing_trace_provider() introduces a behavioral change from the previous logic. Previously, any existing trace provider would be reused regardless of the environment variable settings. Now, an existing provider is only reused if enable_report is true. This means if enable_debug=true but enable_report=false, a new TracerProvider will be created even if one already exists. Document this behavior change or consider whether the condition should be (enable_report or enable_debug) and _has_existing_trace_provider() to preserve the original behavior of reusing any existing provider.

Suggested change
if enable_report and _has_existing_trace_provider():
# Reuse an existing tracer provider whenever tracing is enabled
# (report or debug), preserving the original behavior of reusing
# any previously configured provider.
if (enable_report or enable_debug) and _has_existing_trace_provider():

Copilot uses AI. Check for mistakes.
return ot_trace.get_tracer("agentscope_runtime")
resource = Resource(
attributes={
SERVICE_NAME: _get_service_name(),
SERVICE_VERSION: os.getenv("SERVICE_VERSION", "1.0.0"),
"source": "agentscope_runtime-source",
},
)
provider = TracerProvider(resource=resource)
if _str_to_bool(os.getenv("TRACE_ENABLE_REPORT", "false")):
span_exporter = BatchSpanProcessor(
OTLPSpanGrpcExporter(
endpoint=os.getenv("TRACE_ENDPOINT", ""),
headers=f"Authentication="
f"{os.getenv('TRACE_AUTHENTICATION', '')}",
),
)
provider.add_span_processor(span_exporter)

if _str_to_bool(os.getenv("TRACE_ENABLE_DEBUG", "false")):
span_logger = BatchSpanProcessor(ConsoleSpanExporter())
provider.add_span_processor(span_logger)
# Create TracerProvider if either report or debug is enabled
if enable_report or enable_debug:
resource = Resource(
attributes={
SERVICE_NAME: _get_service_name(),
SERVICE_VERSION: os.getenv("SERVICE_VERSION", "1.0.0"),
"source": "agentscope_runtime-source",
},
)
provider = TracerProvider(resource=resource)

if enable_report:
span_exporter = BatchSpanProcessor(
OTLPSpanGrpcExporter(
endpoint=os.getenv("TRACE_ENDPOINT", ""),
headers=f"Authentication="
f"{os.getenv('TRACE_AUTHENTICATION', '')}",
),
)
provider.add_span_processor(span_exporter)

if enable_debug:
span_logger = BatchSpanProcessor(ConsoleSpanExporter())
provider.add_span_processor(span_logger)
else:

Copilot AI Jan 14, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "Create TracerProvider if either report or debug is enabled" which accurately describes the condition on line 945. However, consider adding a comment for the else block at line 968-969 to clarify that a NoOpTracerProvider is used when tracing is completely disabled, as this is an important aspect of the initialization logic.

Suggested change
else:
else:
# Use a NoOpTracerProvider when both reporting and debug tracing are disabled

Copilot uses AI. Check for mistakes.
provider = ot_trace.NoOpTracerProvider()

tracer = ot_trace.get_tracer(
"agentscope_runtime",
Expand Down
Loading