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

[Feature]: logger access and control #11

Open
2 tasks done
ringerc opened this issue Jun 25, 2024 · 1 comment
Open
2 tasks done

[Feature]: logger access and control #11

ringerc opened this issue Jun 25, 2024 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@ringerc
Copy link
Contributor

ringerc commented Jun 25, 2024

Is there an existing issue already for this feature request/idea?

  • I have searched for an existing issue, and could not find anything. I believe this is a new feature request to be evaluated.

What problem is this feature going to solve? Why should it be added?

The plugin helper machinery hides the zap.Logger and all means of configuring it: It doesn't hook the Zap CLI options nor any env-vars. No means to control log format, json logging etc is provided.

There's only the plugin --debug flag.

Describe the solution you'd like

It would be helpful to be able to provide a plugin startup hook that can configure the logger etc, overriding the defaults here.

Otherwise each entrypoint into the plugin implementation has to reconfigure logging, and still can't influence the logging done by the helper itself.

Describe alternatives you've considered

I didn't really find any, since there doesn't seem to be a sensible means of unwrapping the logr.Logger to obtain the underlying zap root logger.

Additional context

No response

Are you willing to actively contribute to this feature?

No

Code of Conduct

  • I agree to follow this project's Code of Conduct
@ringerc ringerc added the enhancement New feature or request label Jun 25, 2024
@ringerc ringerc changed the title [Feature]: [Feature]: logger access and control Jun 25, 2024
@ringerc
Copy link
Contributor Author

ringerc commented Jul 24, 2024

I fixed this in #31 . It's now possible to do this by injecting a logger into the Cobra command's initial Context, using something like this in cmd/plugin/plugin.go's NewCmd():

        // Replace the pluginhelper's logging implementation with one that
        // injects a stack on panic.
        ctx := cmd.Context()
        if ctx == nil {
                ctx = context.Background()
        }
        cmd.SetContext(newLogContext(ctx))

where newLogContext might be something like

func newLogContext(ctx context.Context) context.Context {
        zc := zap.NewProductionConfig()
        zc.Level = zap.NewAtomicLevelAt(zap.DebugLevel)
        if viper.GetBool("debug") {
                zc.Development = true
                zc.Encoding = "console"
                zc.DisableStacktrace = false
        } else {
                zc.Level.SetLevel(zap.InfoLevel)
                zc.Encoding = "json"
        }
        z, err := zc.Build()
        if err != nil {
                panic(fmt.Sprintf("while building logger: %v", err))
        }
        r := zapr.NewLogger(z)
        r = r.WithName("plugin")
        return logging.IntoContext(ctx, r)
}

It might not be the cleanest approach, but it works. It could use documentation, so I won't close the ticket now, the helper likely needs a hint about how to do it added somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants