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

profiler: Debug log output stream is not configurable #8089

Closed
Bo0mer opened this issue Jun 14, 2023 · 4 comments · Fixed by #8104
Closed

profiler: Debug log output stream is not configurable #8089

Bo0mer opened this issue Jun 14, 2023 · 4 comments · Fixed by #8104
Assignees
Labels
api: cloudprofiler Issues related to the Cloud Profiler API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@Bo0mer
Copy link

Bo0mer commented Jun 14, 2023

Currently when DebugLogging is set to true the profiler package always logs to stderr.

		err = profiler.Start(profiler.Config{
			ProjectID:      cfg.GoogleProjectID,
			Service:        "my-service",
			ServiceVersion: buildInfo.Main.Version,
			DebugLogging:   true,
		})

Produced error messages:

Screenshot 2023-06-07 at 13 04 53

Do you think it makes sense to expose the internally used logger by any means? This way one will able to configure a different output stream for the debug log messages.

@Bo0mer Bo0mer added the triage me I really want to be triaged. label Jun 14, 2023
@product-auto-label product-auto-label bot added the api: cloudprofiler Issues related to the Cloud Profiler API. label Jun 14, 2023
@noahdietz noahdietz added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. needs more info This issue needs more information from the customer to proceed. and removed triage me I really want to be triaged. labels Jun 14, 2023
@noahdietz
Copy link
Contributor

@Bo0mer let's try to be a little more specific with the desired requirements here.

My guess is that you'd like the logger to send debug logs to a different file descriptor/stream/buffer that isn't os.Stderr. That sounds reasonable! I don't think we need to expose the entire logger to achieve this, nor do I think we should just yet.

If the only requirement is to be able to direct logging to a different output, then we can just add a field LogOutput io.Writer that defaults to os.Stderr to the profiler.Config.

@noahdietz noahdietz self-assigned this Jun 14, 2023
@Bo0mer
Copy link
Author

Bo0mer commented Jun 15, 2023

@noahdietz sorry for not being specific enough. Your suggestion totally makes sense and in fact that's what I'm trying to achieve. I want to get the debug logs on stdout, so they're not treated as error severity by the platform I'm using to collect and explore the log messages produced by the application.

Having the LogOutput field in the profiler.Config would be totally sufficient.

Thanks for your fast response.

@noahdietz noahdietz removed the needs more info This issue needs more information from the customer to proceed. label Jun 15, 2023
@noahdietz
Copy link
Contributor

Sure thing, easy enough. I've got a PR open.

@noahdietz
Copy link
Contributor

I'm sorry this shouldn't have been closed until released. I don't typically own this module's release, but I am cutting this one #7555 to get this and another fix out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: cloudprofiler Issues related to the Cloud Profiler API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants