Demo for distributed tracing between react frontend and FastAPI #4
Conversation
| @asynccontextmanager | ||
| async def lifespan(_app: FastAPI): | ||
| global http_client, openai_client | ||
| async with AsyncClient() as _http_client: |
There was a problem hiding this comment.
I think rather simplify the code and just create the client when needed instead of this lifespan stuff
There was a problem hiding this comment.
I think this is fine, it matches my code.
| traceUrl: url.toString(), | ||
| serviceName: 'image-generator-frontend', | ||
| serviceVersion: '0.1.0', | ||
| // for development purposes, we want to see traces as soon as they happen, |
There was a problem hiding this comment.
I think clarify that maxExportBatchSize is the part that means that they will be sent immediately without batching. This also makes scheduledDelayMillis redundant.
distributed-frontend-example/main.py Outdated
| return GenerateResponse(next_url=f'/static/{path}') | ||
| | ||
| # Proxy to Logfire for client traces from the browser | ||
| @app.api_route('/client-traces', methods=['POST', 'OPTIONS']) |
There was a problem hiding this comment.
Added a clarification comment: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Methods/OPTIONS#preflighted_requests_in_cors.
There was a problem hiding this comment.
Doesn't the CORS middleware take care of this?
There was a problem hiding this comment.
you're right, it does.
| Dunno why I can't unresolve #4 (comment) |
| @asynccontextmanager | ||
| async def lifespan(_app: FastAPI): | ||
| global http_client, openai_client | ||
| async with AsyncClient() as _http_client: |
There was a problem hiding this comment.
I think this is fine, it matches my code.
| logfire_token = os.getenv('LOGFIRE_TOKEN') | ||
| logfire_base_url = os.getenv('LOGFIRE_BASE_URL') | ||
| | ||
| assert logfire_token is not None, 'LOGFIRE_TOKEN is not set' | ||
| assert logfire_base_url is not None, 'LOGFIRE_BASE_URL is not set' |
There was a problem hiding this comment.
| logfire_token = os.getenv('LOGFIRE_TOKEN') | |
| logfire_base_url = os.getenv('LOGFIRE_BASE_URL') | |
| assert logfire_token is not None, 'LOGFIRE_TOKEN is not set' | |
| assert logfire_base_url is not None, 'LOGFIRE_BASE_URL is not set' |
Let's get rid of this stuff.
| # Proxy to Logfire for client traces from the browser | ||
| @app.post('/client-traces') | ||
| async def client_traces(request: Request): | ||
| async with httpx.AsyncClient() as client: |
There was a problem hiding this comment.
the existing client is instrumented which isn't desirable
There was a problem hiding this comment.
I'm not really a fan of how the vite react starter script creates 3 (!!!) tsconfig files, can we reduce to just one. I think it's fine for vite.config.ts to be vite.config.js.
There was a problem hiding this comment.
I think we could remove this and just put it all in the root .gitignore
Rather than directly modifying the FastAPI demo, I decided to copy it, since the setup is sufficiently more complicated.
Check the README.md in the PR for more details.
Here's a loom that shows the app/tracing in action https://www.loom.com/share/35a827666b864bed99a4b06dab5c7956?sid=a5e5fdc9-263d-48c6-95ae-9bb9621977b8