-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
"wsk activation poll" should prefix log statements with activation ID #21
Comments
I don't see "everything is mixed up". I see all the logs for each activation id printed together in a group. Am I missing something? |
The current output does not provide the text in brackets |
I'm not sure I want to see a redundant [ activationId] to each line by default. As a human I find the current format easy to read, as the groups are visually obvious. Is the intent of this request to ease machine parsing or human consumption? |
note that the text in brackets was not the activation id but a cloudant document id - actions do not have access to the activation id AFAIK. Take this other output, there are multiple activations running in parallel and in this output I had not yet instrumented my logs to add the document id between brackets:
I don't think the groups are obvious. Today if something goes wrong in one activation, "wsk activation list" does not give me any clue about which activations failed neither how to link it to a specific document id. I would have to look at the individual activations - and there could be a lot. So my best tool today is "wsk activation poll" with the "since" parameter to retrieve the last hours/days, find a trace with my document id and try to identify the activation id from there. |
@l2fprod In your instrumented logs, the id between brackets is an id that is meaningful to you (it is from your action), not an activation id. @sjfink's point is that the log messages in the poll output already demarcates the log activations and prefixing every log message with that id is redundant. I like your suggesting of reporting a better summary in activation list: status (success or failure), and maybe activation duration (discussed in #20). |
so I missed something in how "activation poll" works. It does not work like "tail" showing all the logs from all activations as they happen. Rather it logs an activation ID and its logs when it completes. and in this case there is proper grouping. What is weird is that in the output I pasted this is not happening and you can see several "Activation: name (id)" lines without logs in between and I think I found why: Consider this badly written action where the calls to whisk.done/async are commented
create the action
enable the poll
submit a bunch of them
this is the output I got:
now if I send some more:
and I think that is what happened in the logs I put at the beginning of this ticket. I was not correctly calling whisk.async/done/error and this seems to have an impact on the logging output. |
Yes, this is exactly as I would expect. Since you did not return whisk.async(), technically the activation ended at the return statement, and the asynchronous log was attributed to the next activation. This is by design and is a feature and not a bug. Now that we agree that logs are not interleaved for correct actions, but grouped as intended, do you still think we should have the activation id on each line? |
Provided the developer correctly calls whisk.async/done, the grouping works the purpose - no need for the activation id. However the fact that logs done in the context of one activation gets attached to the next activation because I did not call whisk.async() looks like an unexpected side-effect. Now I have an activation record that has captured the execution logs of multiple action invocations. This sounds like a bug. |
The programming model specifies that if you leave background processing running when you finish an activation, the activity will be charged to the next activation, if there's an activation in the same container. This is pretty fundamental -- in any case it is certainly a feature and not a bug. Apart from that, it appears that we agree that grouping works the purpose and there's no need to print the activation id on each line. So closing as "won't fix". |
@sjfink i tend to agree that from a user perspective, this behavior might be perceived as causing undesired side-effects / inconsistent behavior. eg would the 'charging to the next activation' only be done if the same container gets hit? eg would there be no charging if a different container gets hit? might cause the user to be puzzled on why charging is sometimes done and in other cases not. a more consistent behavior would in my mind be if the container was terminated after a defined timeout window -- or, if the user ends up being in the happy position that his process continued running in the background since the container was cached, then there wouldn't be any charging for the period of time while the container was in the cache. |
@mbehrendt note that no container will run at all in the background -- "idle" containers are paused, no matter whether the user tries to spawn a background process or not. The user will never see charges for any activity except for the normal "synchronous" activation lifetime. The user is never charged while the container is idle in a cache. |
@sjfink i was referring to this stmt from you: " if you leave background processing running when you finish an activation, the activity will be charged to the next activation, if there's an activation in the same container." this sounds to me like the user is charged on the next activation based on a running background process. if not, what is the right way of interpreting this? |
Well, the user is being charged on the next activation based on the time spent in the next activation. It's true that a runaway background process will also be running, and that might slow down the work during the next activation -- but it's an indirect effect -- the user will not be charged directly for the runaway background activity. |
thank you, that now makes sense to me. however, this part of your stmt is still unclear to me and i'm struggling to understand how it fits with your last comment: "the activity will be charged to the next activation, if there's an activation in the same container." . what do you mean by that? |
Add --feed argument support to 'trigger create' command
…e events (apache#21) * initial * rename variable * adapt test * remove unused import * enhance log * correct and enhance test * enhance comments * distinguish between create and update * remove debug code * correct configuration * add missing import * correct type conversion * rename get to read * adapt tests for read events Co-authored-by: ruediger-maass <ruediger.maass@.ibm.com>
Having the activation ID in all log statements will make it easier when debugging an execution as one could sort the rows to group them by activation ID.
Currently everything is mixed up:
Note that in this output, I have prefixed all my log statements with the id of the cloudant document I process - it helps.
The text was updated successfully, but these errors were encountered: