gateway icon indicating copy to clipboard operation
gateway copied to clipboard

feat(telemetry): add Mix format type for access logs

Open codefromthecrypt opened this issue 1 month ago • 2 comments

What type of PR is this?

API enhancement for OpenTelemetry access log configuration.

What this PR does / why we need it:

This PR adds a Mix format type to the access log settings, enabling OTLP access logs to have both body and attributes populated simultaneously.

The OpenTelemetry LogRecord spec has separate body and attributes fields that serve complementary purposes:

  • body: The primary log message (e.g., "%REQ(:METHOD)% %REQ(:PATH)% %PROTOCOL% %RESPONSE_CODE%")
  • attributes: Structured metadata for querying/filtering (e.g., response_code_details, upstream_host)

These fields are NOT mutually exclusive - they're meant to be used together. This is the pattern used by:

The current EG API uses a setting-level format field with Text or JSON types that forces choosing EITHER body (from format.text) OR attributes (from format.json), never both.

The new Mix format type allows both fields to be set:

accessLog:
  settings:
    - format:
        type: Mix
        text: "%REQ(:METHOD)% %REQ(:PATH)% %PROTOCOL% %RESPONSE_CODE%"
        json:
          response_code_details: "%RESPONSE_CODE_DETAILS%"
          upstream_host: "%UPSTREAM_HOST%"
      sinks:
        - type: OpenTelemetry
          openTelemetry:
            host: localhost
            port: 4317

When type: Mix is used:

  • text field becomes the OTLP LogRecord body (primary log message)
  • json field becomes OTLP LogRecord attributes (structured metadata)

Which issue(s) this PR fixes:

N/A - enhancement

Release Notes: Yes

codefromthecrypt avatar Dec 11 '25 06:12 codefromthecrypt

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 72.39%. Comparing base (26a7734) to head (a42c6f6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7720      +/-   ##
==========================================
+ Coverage   72.38%   72.39%   +0.01%     
==========================================
  Files         234      234              
  Lines       34566    34569       +3     
==========================================
+ Hits        25020    25027       +7     
+ Misses       7757     7754       -3     
+ Partials     1789     1788       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Dec 11 '25 06:12 codecov[bot]

@arkodg tl;dr; on this branch. I'm resolving a TODO to support Mix of text (body in otel) and json (attrs in otel) in order to support structured log exports.

The specific use case is Goose which has a session ID for a workflow like "review all my pull requests" and that ID is stored as the attribute "session.id". Until a change like this merges, otel native export will not see the attributes only the raw message. This means you cannot look for all logs for a particular session unless you use a parsing state to try to extract session ID from the log message.

Originally, I thought since this is otel maybe do it at the sink level, but @zirain was right to do it at format is least confusing, The slight noun mismatches (json vs attributes) is ok imho vs defining new fields to discriminate (e.g. new format Structured with fields that exact match otel).

Since we're both looking for design parity and pragmatism here, I think it is basically your call. Looking forward to moving this in whatever shape, so I don't mind refactoring again, if I'm off.

codefromthecrypt avatar Dec 12 '25 08:12 codefromthecrypt

@arkodg @zirain any suggestions on this? It is the last essential part of the dependencies of #7674 and allows otel to work properly wrt access logs. Again, I chose the Mix name to defer to the TODO in hopes of moving this forward (allowing both text and attributes passed to otel logs, not just one or the other)

codefromthecrypt avatar Dec 15 '25 23:12 codefromthecrypt

can you add some CEL test for this? otherwise LGTM

zirain avatar Dec 17 '25 03:12 zirain

@zirain I think I got the tests desired. lemme know if not

codefromthecrypt avatar Dec 17 '25 05:12 codefromthecrypt

/test

zirain avatar Dec 18 '25 02:12 zirain

instead of adding a Type , since the use case is to specify both, Type ProxyAccessLogFormatType could be made optional with Type *ProxyAccessLogFormatType , and CEL could be used to enforce that Text and JSON can only be specified for the OpenTelemetry sink

arkodg avatar Dec 19 '25 04:12 arkodg