flagd icon indicating copy to clipboard operation
flagd copied to clipboard

refactor: WIP: #1324 Slog refactor

Open djosephsen opened this issue 2 months ago • 2 comments

This PR

The intent of this pr is to action #1324 by moving the existing concrete logger type, which is implementation-dependent upon uber-go/zaplog, into an interface type which may be implemented by either go-native slog, or uber-go/zap (or any other logging subysystem the project may become enamored with in the future)

Included is the Interface, along with implementations for both the pre-existing zap-based logger, as well as an interface implementation for slog. The default constructor is set to use the slog subsystem. We have included a full suite of table-based tests for the slog implementation, to prove correctness and demonstrate the logger's use.

Related Issues

#1324

Notes

This is a work-in-progress PR intended to begin the conversation about how the eventual logging refactor should take place. (Adam and I plan to be available at the community meeting to discuss)

Follow-up Tasks

TODOs: There are several places in the current flagd codebase where uber-go/zap is explicitly used by packages in a way that make the project dependent on zap, even outside the current concrete "logger" package, which was intended to abstract these away. These still need to be cleaned up, but that clean-up will depend on the direction the project decides to go for the final implementation of this interface. Examples:

  • Every method invocation to a logger that uses WithFields currently passes in an upstream type called a zap.Field. We can clean this up in a number of ways. The current interface replaces WithField with a method called With which takes variadic any and can be extended to support zap.Field if we want to keep it.

  • Various Tests in the telemetry builder and kubernetes packages create zapcore.Observer types which act like write buffers, enabling the tests to log into the buffer and then examine the buffer's content. The newly added slogger implementation simply takes an io.Writer instead, and can provide the same functionality, but we felt going through and fixing all of those tests was a tad beyond the scope of this WIP. When we're satisfied that this is the correct approach, we'll clean up those tests accordingly (90% of the test coverage for simple cases is already fixed in this PR).

How to test

We've included unit tests for core/pkg/logger. These compile and run. The project also compiles and runs with the new default Logger interface with an slog implementation backing it.

djosephsen avatar Sep 19 '25 22:09 djosephsen

Deploy Preview for polite-licorice-3db33c canceled.

Name Link
Latest commit ea7bc6437e1ecfa3ca49ec2c65ac1b7660ed4626
Latest deploy log https://app.netlify.com/projects/polite-licorice-3db33c/deploys/68cddb6c08f79500083e80b1

netlify[bot] avatar Sep 19 '25 22:09 netlify[bot]

Summary of Changes

Hello @djosephsen, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request initiates a significant refactoring of the project's logging infrastructure by introducing a logger.Logger interface. This change aims to decouple the application from specific logging implementations, allowing for greater flexibility and future extensibility. It includes new implementations for both Go's native log/slog (now the default) and a compatibility layer for the existing uber-go/zap logger. While many components have been updated to use the new interface, the PR is a work-in-progress, with identified follow-up tasks to fully address the handling of zap.Field types when using the slog backend and to refactor existing tests that rely on zapcore.Observer.

Highlights

  • Logger Interface Introduced: A new logger.Logger interface has been defined to abstract logging implementations, moving away from a concrete uber-go/zap type.
  • slog Integration: A new slogger implementation based on Go's native log/slog package has been added, and is now the default logger for new instantiations.
  • zap Compatibility Layer: The existing uber-go/zap logger has been adapted into a zapLogger type that conforms to the new logger.Logger interface.
  • Unified Field Handling: The WithFields method has been generalized to With, now accepting variadic any arguments. This change aims for a more generic approach, though explicit zap.Field types passed to slog implementations are noted as a follow-up for proper handling.
  • Widespread Adoption: The new logger.Logger interface and its New and With methods have been integrated across various core and service packages, updating field types and constructor calls.
  • New Test Suite: Comprehensive unit tests have been introduced for the slog implementation to validate its functionality and ensure correctness.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

[^1]: Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

gemini-code-assist[bot] avatar Sep 19 '25 22:09 gemini-code-assist[bot]