-
Notifications
You must be signed in to change notification settings - Fork 148
[DERCBOT-1374] Public URL for Langfuse Observability Settings #1856
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
[DERCBOT-1374] Public URL for Langfuse Observability Settings #1856
Conversation
2f1f3f2
to
0611d9e
Compare
7d54ad9
to
b94864d
Compare
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.
Thanks for your code, it just needs a few adjustments.
...strator-server/src/main/python/server/src/gen_ai_orchestrator/routers/responses/responses.py
Outdated
Show resolved
Hide resolved
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.
Ok but needs to remove it from python side as it's not used
@@ -42,6 +42,11 @@ class LangfuseObservabilitySetting(BaseObservabilitySetting): | |||
url: AnyUrl = Field( | |||
description='The Langfuse server url', examples=['https://cloud.langfuse.com'], default='http://localhost:3000' | |||
) | |||
public_url: Optional[AnyUrl] = Field( |
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.
Why do you have a public URL here as it's not used in the orchestrator.
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 see we share the same data model between the orchestrator and the bot admin BotObservabilityConfigurationDTO.setting is LangfuseObservabilitySetting which came from the Orchestrator API contract we should have a BotAdmin API contract decorrelated from the ochestrator contract to avoid this.
f73351e
to
e5a2f62
Compare
Ticket : DERCBOT-1374
This PR introduces a new optionnal attribute for the Observability Settings :
If filled, the traceUrl of
ObservabilityInfo
will get replaced by the public URL as so :