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

Show update handler and ID in logging context #466

Open
6 tasks done
drewhoskins-temporal opened this issue Apr 24, 2024 · 6 comments
Open
6 tasks done

Show update handler and ID in logging context #466

drewhoskins-temporal opened this issue Apr 24, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@drewhoskins-temporal
Copy link

drewhoskins-temporal commented Apr 24, 2024

Is your feature request related to a problem? Please describe.

It's hard to tell when log lines occurred within an update (or signal, or query) handler.

Describe the solution you'd like

I logged from within an update handler in the python SDK with self.logger.info
Imagining I saw this line in production, and I didn’t build the workflow, I could be confused as to why this code is getting executed. In fact, my update handler can also be called by the workflow itself, meaning it'd be nice if the context clarified whether I'm in an update handler or the main workflow.

Or I might want to grep the logs for all the times when an update/signal/query was called.

INFO:batch_orchestrator:This is my log message. ({'attempt': 1, 'namespace': 'default', 'run_id': '183ba64c-a415-492a-b569-a0e97d1a3114', 'task_queue': 'my-task-queue', 'workflow_id': 'inflate_product_prices-37acbbae-d941-4be8-8242-56f15a91d892', 'workflow_type': 'BatchOrchestrator'})

I'd love if the context or prefix listed which handler it's in somehow and perhaps whether it was a signal/update/query. In the case of an update, I'd like to see the update ID.

Additional context

Per-SDK Tickets

@drewhoskins-temporal drewhoskins-temporal added the enhancement New feature or request label Apr 24, 2024
@Quinn-With-Two-Ns
Copy link
Contributor

Imagining I saw this line in production, and I didn’t build the workflow, I could be confused as to why this code is getting executed.

If you didn't write the workflow how will knowing what handler the log came from really help? What if a user wants to know all the logs that came from a particular update request? It doesn't help with query or signal, but we could consider adding the updateID as another KV in the logger.

@drewhoskins-temporal
Copy link
Author

drewhoskins-temporal commented Apr 24, 2024 via email

@Quinn-With-Two-Ns
Copy link
Contributor

If you had multiple updates how would you tell which one caused your bug if all you have is the handle?

@cretz
Copy link
Member

cretz commented Apr 24, 2024

We/users can add logging scope if we need to in some langs, but in other langs that's non-standard. But it's important to understand that code that runs as a result of one of these interactions cannot always know it came from one of these interactions (e.g. a signal in Go is a channel, there's no handler, and e.g. many signal handlers just append something to a list to be processed by the primary coroutine). If it is important for a user to ship an action-specific logger around, they may.

In Python you might:

@workflow.update
async def do_update(self, in: MyUpdateInput) -> MyUpdateOutput:
    workflow.logger.info("This is my log message", extra={"handler": "do_update"})
    ...

^ This is how Python users are expected to add things to loggers. We only do workflow-level contextual information implicitly because the logger is workflow specific. Some languages (not Python) have a concept of scoped loggers, and some languages (Python) expect you to pass contextual information if you want it on your logs.

We can consider adding contextvars for logger, but in Python the common way is to carry state that the log caller sets on the log message or create an entirely new logger with an adapter (which is basically what we do at the workflow level). For other languages, we can discuss their most common way to add scope. But it is very important we are language idiomatic and that all languages approaches are understood well for issues like this.

@drewhoskins-temporal
Copy link
Author

Max is bought in to doing something here. I'm tagging it as part of update GA.

@sync-by-unito sync-by-unito bot closed this as completed Jul 12, 2024
@josh-berry josh-berry reopened this Jul 13, 2024
@drewhoskins-temporal drewhoskins-temporal changed the title Show update handler in logging context Show update handler and ID in logging context Sep 24, 2024
@drewhoskins-temporal
Copy link
Author

Interesting somewhat-related conversation about tracing with a customer. Because update is synchronous, and datadog doesn't support being part of two traces, they wanted their updates to be traced as part of the caller's traces rather than the workflow's.
I think that may underscore that the user will want to see logs associated with the update operation even moreso than the workflow. Even in the absence of tracing, logging the Update ID could help connect the client to the update handler in debugging scenarios, and the update name will greatly clarify what's going on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants