-
Notifications
You must be signed in to change notification settings - Fork 285
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
feat(traces): configurable endpoint for the exporter #1795
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.
The docstring for HttpExporter
should specify that endpoint
supersedes host
and port
, so there is no need to specify a host
and port
if you've already provided an endpoint
.
src/phoenix/config.py
Outdated
|
||
|
||
def get_env_trace_endpoint() -> Optional[str]: | ||
return os.getenv(ENV_TRACE_ENDPOINT) or None |
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.
return os.getenv(ENV_TRACE_ENDPOINT) or None | |
return os.getenv(ENV_TRACE_ENDPOINT) |
os.getenv
returns None
if the environment variable can't be found.
src/phoenix/config.py
Outdated
@@ -7,6 +7,7 @@ | |||
ENV_PHOENIX_PORT = "PHOENIX_PORT" | |||
ENV_PHOENIX_HOST = "PHOENIX_HOST" | |||
ENV_NOTEBOOK_ENV = "PHOENIX_NOTEBOOK_ENV" | |||
ENV_TRACE_ENDPOINT = "PHOENIX_TRACE_ENDPOINT" |
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.
keep it generic?
ENV_TRACE_ENDPOINT = "PHOENIX_TRACE_ENDPOINT" | |
ENV_PHOENIX_RECEIVER_ENDPOINT = "PHOENIX_RECEIVER_ENDPOINT" |
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.
Oh yeah evals. Was copying OTEL naming but maybe I'll just call it collector
resolves #1793
This PR introduces an
endpoint
config for the exporter so that we don't assume that the phoenix server is running locally. This is needed for various use-cases:Phoenix can now be running on an EC2 instance or other
Phoenix might have to be running in Sagemaker studio in a separate process (Networking failing right now but in theory this might be the configuration)