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: add loki compatible logging output formatting #30

Merged
merged 1 commit into from
Dec 19, 2024
Merged

Conversation

prom3theu5
Copy link
Member

@prom3theu5 prom3theu5 commented Dec 19, 2024

PR Type

Enhancement


Description

  • Added support for Grafana Loki-compatible logging output formatting
  • Introduced new parameters to control log output format:
    • lokiCompatible: enables Loki-compatible output
    • rawCompactJson: toggles between compact JSON and Loki JSON text format
  • Added Serilog.Sinks.Grafana.Loki package and its dependencies
  • Updated various package versions including OpenTelemetry and Serilog components

Changes walkthrough 📝

Relevant files
Enhancement
OtlpServiceExtensions.cs
Add Loki logging format support to OTLP service                   

src/SimCube.Aspire/Features/Otlp/OtlpServiceExtensions.cs

  • Added support for Loki-compatible logging output formatting
  • Added new parameters lokiCompatible and rawCompactJson to control log
    output format
  • Modified logger configuration to support different output formatters
  • +20/-7   
    GlobalUsings.cs
    Add Loki-related global usings                                                     

    src/SimCube.Aspire/GlobalUsings.cs

  • Added global usings for Serilog.Formatting.Compact
  • Added global usings for Serilog.Sinks.Grafana.Loki
  • +2/-0     
    Dependencies
    Directory.Packages.props
    Add Loki package and update dependencies                                 

    Directory.Packages.props

  • Added Serilog.Sinks.Grafana.Loki package
  • Updated various package versions
  • Reorganized package structure
  • +42/-41 
    SimCube.Aspire.csproj
    Add Loki sink package reference                                                   

    src/SimCube.Aspire/SimCube.Aspire.csproj

    • Added Serilog.Sinks.Grafana.Loki package reference
    +1/-0     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Null Reference
    The configuration value for OtlpLiterals.ServiceName is accessed without null check which could cause runtime exception

    Code Duplication
    The default console output format is checked and set twice - once in the parameter default and again in the if condition

    Error Handling
    The switch expression for lokiCompatible formatting doesn't handle potential null cases from the formatters

    @prom3theu5 prom3theu5 merged commit d32a3ed into main Dec 19, 2024
    2 checks passed
    @prom3theu5 prom3theu5 deleted the dev branch December 19, 2024 12:04
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add null check for configuration value to prevent runtime exceptions

    Add null check for the configuration[OtlpLiterals.ServiceName] to prevent potential
    runtime exceptions if the service name is not configured.

    src/SimCube.Aspire/Features/Otlp/OtlpServiceExtensions.cs [49]

    -.Enrich.WithProperty(nameof(OtlpLiterals.ServiceName), configuration[OtlpLiterals.ServiceName]);
    +.Enrich.WithProperty(nameof(OtlpLiterals.ServiceName), configuration[OtlpLiterals.ServiceName] ?? "UnknownService");
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses a potential runtime exception by adding a null check for the service name configuration. This is a critical defensive programming practice that can prevent application crashes.

    8
    General
    Extract complex configuration logic into a separate method for better maintainability

    Move the switch expression to a separate method to improve code readability and
    maintainability, as it handles complex logging configuration logic.

    src/SimCube.Aspire/Features/Otlp/OtlpServiceExtensions.cs [51-55]

    -config = lokiCompatible switch
    -{
    -    true => config.WriteTo.Console(rawCompactJson ? new RenderedCompactJsonFormatter() : new LokiJsonTextFormatter()),
    -    false => config.WriteTo.Spectre(outputTemplate: consoleOutputFormat),
    -};
    +config = ConfigureLogOutput(config, lokiCompatible, rawCompactJson, consoleOutputFormat);
     
    +private static LoggerConfiguration ConfigureLogOutput(
    +    LoggerConfiguration config,
    +    bool lokiCompatible,
    +    bool rawCompactJson,
    +    string consoleOutputFormat) =>
    +    lokiCompatible
    +        ? config.WriteTo.Console(rawCompactJson ? new RenderedCompactJsonFormatter() : new LokiJsonTextFormatter())
    +        : config.WriteTo.Spectre(outputTemplate: consoleOutputFormat);
    +
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The suggestion improves code organization and readability by extracting complex logging configuration logic into a dedicated method. While beneficial for maintainability, it's more of a code structure improvement than a critical fix.

    5

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant