Skip to content

fix: some telemetry APIs don't currently work#1188

Merged
ehhuang merged 1 commit intomainfrom
pr1188
Feb 20, 2025
Merged

fix: some telemetry APIs don't currently work#1188
ehhuang merged 1 commit intomainfrom
pr1188

Conversation

@ehhuang
Copy link
Copy Markdown
Contributor

@ehhuang ehhuang commented Feb 20, 2025

Summary:

This bug is surfaced by using the http LS client. The issue is that non-scalar values in 'GET' method are body params in fastAPI, but our spec generation script doesn't respect that. We fix by just making them POST method instead.

Test Plan:
Test API call with newly sync'd client (llamastack/llama-stack-client-python#149)

image

Summary:

This bug is surfaced by using the http LS client. The issue is that non-scalar values in 'GET' method are `body` params in fastAPI, but our spec generation script doesn't respect that. We fix by just making them POST method instead.

Test Plan:
Test API call with newly sync'd client (llamastack/llama-stack-client-python#149)
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Feb 20, 2025

@webmethod(route="/telemetry/spans/{span_id:path}/tree", method="GET")
@webmethod(route="/telemetry/spans/{span_id:path}/tree", method="POST")
async def get_span_tree(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: can we rename this query_span_tree? even the response type is QuerySpanTreeResponse ... having a get_ method be called POST is causing me a bit of dissonance. "query_" makes it slightly more palatable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sg. Let me do the renaming in a followup

@ehhuang ehhuang merged commit 1166afd into main Feb 20, 2025
4 checks passed
@ehhuang ehhuang deleted the pr1188 branch February 20, 2025 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants