-
Notifications
You must be signed in to change notification settings - Fork 1.2k
chore!: remove telemetry API usage #3815
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think still needed internally to keep the otel logging
e3ab49c
to
9d244b4
Compare
llama_stack/core/resolver.py
Outdated
"opentelemetry-sdk", | ||
"opentelemetry-exporter-otlp-proto-http", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should add these deps to pyproject.toml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea! let me do that instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like they're already in SERVER_DEPENDENCIES
. I think that is good enough since those are included in every build
?
2ba8570
to
e9c7784
Compare
remove telemetry as a providable API from the codebase. This includes removing it from generated distributions but also the provider registry, the router, etc relates to llamastack#3806 Signed-off-by: Charlie Doern <cdoern@redhat.com>
service_name: "${env.OTEL_SERVICE_NAME:=\u200B}" | ||
sinks: ${env.TELEMETRY_SINKS:=} | ||
otel_exporter_otlp_endpoint: ${env.OTEL_EXPORTER_OTLP_ENDPOINT:=} | ||
post_training: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should add telemetry.enabled: true in these configs so the behavior remains unchanged.
# What does this PR do? old telemetry config was removed in #3815 ## Test Plan ❯ OTEL_SERVICE_NAME=aloha OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 uv run llama stack run starter
# What does this PR do? old telemetry config was removed in #3815 ## Test Plan ❯ OTEL_SERVICE_NAME=aloha OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 uv run llama stack run starter
# What does this PR do? leftover from #3815 ## Test Plan CI --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/llamastack/llama-stack/pull/3828). * #3830 * __->__ #3828
# What does this PR do? old telemetry config was removed in #3815 ## Test Plan ❯ OTEL_SERVICE_NAME=aloha OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 uv run llama stack run starter
# What does this PR do? old telemetry config was removed in #3815 ## Test Plan ❯ OTEL_SERVICE_NAME=aloha OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 uv run llama stack run starter <img width="1888" height="605" alt="image" src="https://github.com/user-attachments/assets/dd5cc9f0-213a-4dc6-9385-f61a3a13b4c3" />
What does this PR do?
remove telemetry as a providable API from the codebase. This includes removing it from generated distributions but also the provider registry, the router, etc
since
setup_logger
is tied pretty strictly toApi.telemetry
being in impls we still need an "instantiated provider" in our implementations. However it should not be auto-routed or provided. So in validate_and_prepare_providers (called from resolve_impls) I made it so that if run_config.telemetry.enabled, we set up the meta-reference "provider" internally to be used so that log_event will work when called.This is the neatest way I think we can remove telemetry from the provider configs but also not need to rip apart the whole "telemetry is a provider" logic just yet, but we can do it internally later without disrupting users.
so telemetry is removed from the registry such that if a user puts
telemetry:
as an API in their build/run config it will err out, but can still be used by us internally as we go through this transition.relates to #3806