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

Log output during /actuator/refresh call #1311

Closed
litepl opened this issue Nov 17, 2023 · 11 comments · Fixed by #1357
Closed

Log output during /actuator/refresh call #1311

litepl opened this issue Nov 17, 2023 · 11 comments · Fixed by #1357

Comments

@litepl
Copy link

litepl commented Nov 17, 2023

Is your feature request related to a problem? Please describe.
When an "old-style" reload finds some changes in configmap o secret it puts some info into the log like:

2023-11-16 13:05:26,718 - xxx-cc77b79d5-dfr4l - INFO  [pool-16-thread-1 - trace=N/A, span=N/A] o.s.c.l.LogAccessor: Detected change in config maps

calling /actuator/refresh says nothing in the log

Describe the solution you'd like
It would be nice to have such a log when calling /actuator/refresh endpoint

Describe alternatives you've considered
None

Additional context
None

@wind57
Copy link
Contributor

wind57 commented Nov 17, 2023

I don't know about this... you can write your own listener and catch RefreshScopeRefreshedEvent, right?

@wind57
Copy link
Contributor

wind57 commented Dec 13, 2023

I was not too verbose on the above comment, but IMO, I don't understand what log statement you would want.

When we log: "Detected change in config maps" for example, we know that a change has happened, as such we issue that log statement. But in case of calling /actuator/refresh - what would you expect to happen? Something along the lines of :

refresh endpoint called, reloading configmap/secret based property sources.

If so, then why can't you do it in your app? You will need something along the lines of:

class MyListener implements ApplicationListener<RefreshScopeRefreshedEvent> { 
     @Override
	public void onApplicationEvent(RefreshScopeRefreshedEvent event) {
		// log whatever you want
	}
}

register this one as a @Bean/@Component/etc and you are good. We could probably add it too, but that might be, or not be a good option.

@ryanjbaxter thoughts?

@wind57
Copy link
Contributor

wind57 commented Dec 14, 2023

I don't think that this is what OP is asking for. That log statement still comes when we receive an event from kubernetes about the state of a certain configmap.

What they want is a log statement when /actuator/refresh is called directly, like with curl for example. This is how I read this, at least.

@litepl
Copy link
Author

litepl commented Dec 14, 2023

Exactly. That's because of migration to Spring Cloud Configuration Watcher which calls /actuator/refresh.

@wind57
Copy link
Contributor

wind57 commented Dec 14, 2023

@litepl I think you should have mentioned that in the initial description. We have a bunch of logs from configuration watcher side for this. At the moment they are on debug mode though. For example, this this log statement.

But again, that is in the configuration watcher, not your application. Will that suite your needs?

@litepl
Copy link
Author

litepl commented Dec 14, 2023

Not exactly. To me, it'd be better to have that information in the app's log, not in sckcw ones. It's easier to create alerts for that as an example.

@litepl
Copy link
Author

litepl commented Dec 15, 2023

Yes. Not I want to, I would like to :D It's a kind request

@wind57
Copy link
Contributor

wind57 commented Dec 15, 2023

I am still inclined not to do this. A log statement there does nothing useful, imho; but if someone really needs it, its really easy to write your own Listener as outlined above.

Especially since a call to /refresh might come from some other caller, not only the configuration watcher. We can't even distinguish where the call comes from, as such the log statement can not even contain "configmaps" or "secrets" words.

We do have proper logs that would make far more sense and they come from the configuration watcher, from the source. If alerts can be created on the app, then Im sure alerts can be created on the watcher also.

This is my view on this issue, at least at the moment :)

@ryanjbaxter
Copy link
Contributor

Well this is not the repo for the request as the code that handles the /refresh endpoint lives in Spring Cloud Commons. It is not specific to Kubernetes its any Spring Cloud application. If you would like, you can submit a PR to add the log statement and we can see what the rest of the team thinks.

@ryanjbaxter ryanjbaxter transferred this issue from spring-cloud/spring-cloud-kubernetes Dec 15, 2023
@ryanjbaxter ryanjbaxter linked a pull request Jun 13, 2024 that will close this issue
@ryanjbaxter ryanjbaxter added this to the 4.1.4 milestone Jun 13, 2024
@github-project-automation github-project-automation bot moved this to Done in 2023.0.3 Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants