Skip to content
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

Run /metrics on another port #273

Closed
severo opened this issue May 16, 2022 · 5 comments
Closed

Run /metrics on another port #273

severo opened this issue May 16, 2022 · 5 comments

Comments

@severo
Copy link
Collaborator

severo commented May 16, 2022

See #260 (comment)

See also how it's done in Go for the tensorboard launcher: https://github.com/huggingface/tensorboard-launcher/blob/46df45821e8311095e824bc39b9acdadaf99634c/launcher/pkg/cmd/launcher/main.go#L61

@severo
Copy link
Collaborator Author

severo commented May 16, 2022

Note that one issue is that we want to get metrics about the main app using the PrometheusMiddleware (https://github.com/huggingface/datasets-server/blob/cc763a31d588b88ce56a0ac6eccc6fd923a8a9eb/services/api/src/api/app.py#L48). Running the /metrics endpoint on a totally separate process would not give access to these metrics. That's why we run the endpoint inside the same app for now. Uvicorn does not (and won't) support binding apps to different ports.

@severo
Copy link
Collaborator Author

severo commented May 16, 2022

An option is to rewrite the api service in another language, maybe Go or Node.js: #493

Another one is to setup the reverse proxy.

@severo
Copy link
Collaborator Author

severo commented Aug 5, 2022

See a report from hackerone here: #496

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@severo
Copy link
Collaborator Author

severo commented Sep 16, 2022

not needed anymore (at least for now). Maybe take into account if we migrate to node (#493)

@severo severo closed this as completed Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant