-
Notifications
You must be signed in to change notification settings - Fork 30
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
Configure loggers for function #1421
base: main
Are you sure you want to change the base?
Conversation
This PR has dependency on #1408 |
@akihikokuroda let me put this PR |
Removing the label now that we released the new version and we can discuss this calmly 😄 |
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 think that with the new catalog client, we may be able to tackle the logging issue there and leave the original client as is (?)
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 like this approach @akihikokuroda , thank you. Some comments that I have about the implementation:
log_type
: I don't think is needed but correct me if I'm wrong, Aki. Backend logic can be modified to identify if the function is fromprovider
oruser
without the need to use this variable.- I see that this is mostly for provider functions. How could we do this for user functions?
- Right now we have some validations in the logs like this one, how can we check these things with this new approach?
The provider can get 3 different logs (user, function and console output).
The user can put the log entries into the different logs and retrieve them separately.
It is using the standard logger function, so if log entries are not there, it returns nothing. |
Well, this workflow is something I would like to validate with Sanket too so let's discuss it internally. Personally I don't see the value to support both kind of logs: user log and console output. Same as that provider would have access to user logs having them their own logs.
For me as user sounds confusing to have two kind of logs that basically do the same.
Before merging anything we need to ensure that we are not loosing any feature like that mentioned. Let me Monday write in our internal issue about this to initiate a conversation with Paco and Sanket. |
The use log entries are generated by the function that are shared with the user. It's still valuable for the provider to check what are shared with the user. |
I agree but I feel a bit uncomfortable doing that so I prefer to confirm that that's exactly what we are looking for. For that is for what we are separating logs but I agree with you in this assumption. Logs management is something that it's not closed yet and I prefer to think well about it taking into account that it's not so urgent.
Can we not redirect that to the logs? Thinking in that at some point as we talked we were going to remove logs from database (something that I think it has a lot of sense). |
That's the original log. We can take it out but it probably contains critical information to debug the function issues. |
Oh yes, I was thinking in that "function" logs not "user" logs. "user" logs should only print that information specifically sent it to it I suppose. |
@Tansito Is there any progress for this? What is the next step? |
Sorry @akihikokuroda I couldn't address this yet. Feel free to open the conversation in our internal issue. I will try to add there my comments. |
Summary
"user" and "function" loggers are created in the runner class
The "user" logger writes the log into /data/{job_id}/user.log file which is /{user_name}/{job_id}/user.log in the persistent storage
The "function" logger writes the log into /function_date/{job_id}/function.log file which is /{provider_name}/{job_id}/function.log in the persistent storage
The function code can get these logger by logger.getLogger("user") and `logger.getLogger("function")
An optional argument type is added to job.logs() method. The 'type value should be "user" or "function".
For the provider, it returns both "user" and "function" logs. For the user, it returns only "user" log
Details and comments