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

feat: print diff on config change #202

Merged
merged 2 commits into from
Jan 28, 2025

Conversation

leseb
Copy link
Contributor

@leseb leseb commented Jan 28, 2025

Commit Message:

feat: print diff on config change

When the configuration file is updated, the logger will print the
updated sections at the DEBUG level. The implementation is
straightforward and does not rely on any external libraries, making it
particularly useful for troubleshooting.

Output:

time=2025-01-28T11:46:03.911+01:00 level=INFO msg="loading a new config" path=docs/extproc-config.yaml
time=2025-01-28T11:46:03.911+01:00 level=DEBUG msg="config line changed" line=14 path=docs/extproc-config.yaml old="weight: 1" new="weight: 0"

Signed-off-by: Sébastien Han [email protected]

Related Issues/PRs (if applicable):

Special notes for reviewers (if applicable):

@leseb leseb requested a review from a team as a code owner January 28, 2025 14:26
@leseb leseb force-pushed the config-change-diff branch from af0b0b1 to 4fc4d10 Compare January 28, 2025 14:28
Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

I like this - minor comment but LGTM

internal/extproc/watcher.go Show resolved Hide resolved
When the configuration file is updated, the logger will print the
updated sections at the DEBUG level. The implementation is
straightforward and does not rely on any external libraries, making it
particularly useful for troubleshooting.

Output:

```
time=2025-01-28T11:46:03.911+01:00 level=INFO msg="loading a new config" path=docs/extproc-config.yaml
time=2025-01-28T11:46:03.911+01:00 level=DEBUG msg="config line changed" line=14 path=docs/extproc-config.yaml old="weight: 1" new="weight: 0"
```

Signed-off-by: Sébastien Han <[email protected]>
@leseb leseb force-pushed the config-change-diff branch from 4fc4d10 to bd9a408 Compare January 28, 2025 16:25
@mathetake
Copy link
Member

@leseb could you not force-push as per CONTRIBUTING.md?

@mathetake
Copy link
Member

also, could you modify the PR description so that it follows the template

Signed-off-by: Sébastien Han <[email protected]>
@leseb leseb force-pushed the config-change-diff branch from dbee65d to 827bf4b Compare January 28, 2025 16:44
@leseb
Copy link
Contributor Author

leseb commented Jan 28, 2025

I had to do another force-push on the second commit since DCO was complaining...

@leseb leseb requested a review from mathetake January 28, 2025 16:46
Copy link
Member

@mathetake mathetake 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!!

@leseb
Copy link
Contributor Author

leseb commented Jan 28, 2025

Thank you!!

Thanks! So when CI is green I'll amend it to have a single final commit and force-push, is that alright?

@mathetake
Copy link
Member

well, all PRs will be squash-merged, so you don't need to do that

@mathetake mathetake merged commit f7b5851 into envoyproxy:main Jan 28, 2025
18 checks passed
@leseb leseb deleted the config-change-diff branch January 28, 2025 16:55
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.

2 participants