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

minimum log_level for GET /../logs #485

Closed
soxofaan opened this issue Mar 27, 2023 · 7 comments · Fixed by #488
Closed

minimum log_level for GET /../logs #485

soxofaan opened this issue Mar 27, 2023 · 7 comments · Fixed by #488
Labels
data processing minor requires a minor-version (x.1.0 for example)
Milestone

Comments

@soxofaan
Copy link
Member

soxofaan commented Mar 27, 2023

#431 added option to specify minimum log_level at job submission time.
We are currently also playing with setting a minimum log_level when requesting the logs (e.g. GET /jobs/{job_id}/logs).
For example see Open-EO/openeo-python-client#332, where we filter client side to only show error logs, but we've experienced that it would be better to do the filtering already back-end side (for performance reasons).

Can we add an optional log_level parameter to GET /jobs/{job_id}/logs and GET /services/{service_id}/logs for back-end side log filtering at retrieval time?

refs and related issues

@m-mohr
Copy link
Member

m-mohr commented Mar 28, 2023

I guess we can do that, but I'm wondering whether it should be a user-selectable list instead, e.g. if a user uses inspect with the debug level and only wants the debug level + maybe errors.

@m-mohr m-mohr added data processing minor requires a minor-version (x.1.0 for example) labels Mar 28, 2023
@soxofaan
Copy link
Member Author

I guess we can do that, but I'm wondering whether it should be a user-selectable list instead,

For back-end side filtering I would just go for a minimum level, which is the most natural way of working with logs.
If some additional filtering is necessary: that can always be done client side (e.g. interactively), but I would not extend that UI aspect to the back-end side.

@m-mohr
Copy link
Member

m-mohr commented Mar 29, 2023

I just fear that this is not fulfilling what some users may want. The user logs (from inspect) usually reside in "info" or "debug" (so low severance) and "error" is high severance. So you can't get the (I assume) most crucial information at once.

@soxofaan
Copy link
Member Author

I don't think there is a problem. By default, a /logs request that is e.g. triggered explicitly by the user will return all logs and the user or client will be able filter down (client side) as desired.

The point of this feature request is to have a standardized way to limit log retrieval backend-side for certain situations where there is no explicit interactive user involvement: e.g. in python client, when a batch job failed, we want to immediately show a relevant error message, instead of just "your job failed, check the logs". This should streamline the user support cycle. Unfortunately we have experienced that returning all logs, just to print the error level ones is very inefficient, a pretty heavy (sometimes even unstable) operation and possibly makes the user experience even worse.

@m-mohr
Copy link
Member

m-mohr commented Mar 30, 2023

Yes, that's indeed a good usecase. Hearing more opinions would be interesting. @dthiex @SerRichard @clausmichele

@clausmichele
Copy link
Member

For the use case that @soxofaan mentioned it seems reasonable to have a flag/parameter to set the level. Other than that, I agree with @m-mohr that if I user asks specifically for the logs the default value should be the lowest possible, so that they can filter them later at the client side.

@m-mohr
Copy link
Member

m-mohr commented Apr 6, 2023

Please review PR #488

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data processing minor requires a minor-version (x.1.0 for example)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants