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

Add custom logger option to schema tool #1943

Merged
merged 4 commits into from
Sep 21, 2021

Conversation

jlegrone
Copy link
Contributor

What changed?

Added a logger configuration option to the schema migration tool. This enables filtering and reformatting of migration logs.

Why?

The existing schema migration logs can be noisy, particularly when they are mixed in with test logs (as is the case in temporalite). This also makes it possible to output structured logs, which are typically easier to integrate with third party log aggregation services.

How did you test it?

Validated by adding a local override for go.temporal.io/server in the go.mod of temporalite and running migrations against sqlite.

Potential risks

The logs produced by the schema tool now default to a different format. This format is determined by the log.NewCLILogger() function.

Is hotfix candidate?

no

@jlegrone jlegrone requested a review from a team September 20, 2021 13:14
tools/common/schema/types.go Show resolved Hide resolved
tools/common/schema/setuptask.go Outdated Show resolved Hide resolved
@jlegrone jlegrone force-pushed the jlegrone/schema-logger branch from cf827a5 to 8fb4b6f Compare September 20, 2021 19:18
@alexshtin
Copy link
Member

So there are no usages of "log" package in tools/common/schema dir, right? This is great. We should remove "log" from everywhere in the project.

@alexshtin alexshtin merged commit d6c9b4a into temporalio:master Sep 21, 2021
@jlegrone jlegrone deleted the jlegrone/schema-logger branch September 21, 2021 14:49
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.

3 participants