Skip to content

jobs: add log tags on contexts for Resume and OnFailOrCancel hooks#45728

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
thoszhang:job-resumer-log-ctx
Mar 5, 2020
Merged

jobs: add log tags on contexts for Resume and OnFailOrCancel hooks#45728
craig[bot] merged 1 commit intocockroachdb:masterfrom
thoszhang:job-resumer-log-ctx

Conversation

@thoszhang
Copy link
Copy Markdown

This PR adds a log tag with the job ID (job=...) to the contexts
passed to Resume() and OnFailOrCancel(), so that logs emitted from
those job hooks can be associated with the job.

We should probably expand the scope of using these log tags to other
parts of the job registry to avoid the overhead of manually including
job IDs in log messages. This is meant to be a starting point and have a
limited, well-defined scope.

Release note (general change): Logs emitted from jobs are now tagged
with the job ID to improve visibility and aid debugging.

@thoszhang thoszhang requested review from dt and spaskob March 4, 2020 21:16
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@thoszhang
Copy link
Copy Markdown
Author

@spaskob I'm proposing this as an alternative to including job: %d to logs in the schema change job resumer that interact with the job. Eventually we may want to replace all the job: %d prefixes with this (or some other means of tagging the logs).

One slight downside is that now there are multiple formats surrounding job IDs in logs, but in practice we should grep for just the job ID itself.

Copy link
Copy Markdown
Contributor

@spaskob spaskob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this is awesome!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dt and @spaskob)

@thoszhang
Copy link
Copy Markdown
Author

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 4, 2020

Build failed (retrying...)

@thoszhang
Copy link
Copy Markdown
Author

bors flake

bors r+

This PR adds a log tag with the job ID (`job=...`) to the contexts
passed to `Resume()` and `OnFailOrCancel()`, so that logs emitted from
those job hooks can be associated with the job.

We should probably expand the scope of using these log tags to other
parts of the job registry to avoid the overhead of manually including
job IDs in log messages. This is meant to be a starting point and have a
limited, well-defined scope.

Release note (general change): Logs emitted from jobs are now tagged
with the job ID to improve visibility and aid debugging.
@thoszhang thoszhang force-pushed the job-resumer-log-ctx branch from 86b89aa to 6f39a24 Compare March 5, 2020 05:00
@thoszhang
Copy link
Copy Markdown
Author

It was the CLA assistant...

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 5, 2020

Build succeeded

@craig craig bot merged commit c9b189b into cockroachdb:master Mar 5, 2020
@thoszhang thoszhang deleted the job-resumer-log-ctx branch April 15, 2020 01:23
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

Successfully merging this pull request may close these issues.

5 participants