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

access_log: add newline between JSON log lines #5024

Merged

Conversation

aa-stripe
Copy link
Contributor

Description:
Tack a newline onto the end of all JSON access log lines

Risk Level:
Low

Testing:
Every JSON unit test will now check to see that there is one (and only one) newline in a log line

Docs Changes:
None

Release Notes:
None

Fixes #5018

Signed-off-by: Aaltan Ahmad <aa@stripe.com>
Signed-off-by: Aaltan Ahmad <aa@stripe.com>
@aa-stripe
Copy link
Contributor Author

@htuch @mattklein123 ptal!

@aa-stripe
Copy link
Contributor Author

Verified manually with this config:

admin:
  access_log_path: /tmp/admin_access.log
  address:
    socket_address: { address: 0.0.0.0, port_value: 9901 }
static_resources:
  listeners:
  - name: listener_0
    address:
      socket_address: { address: 0.0.0.0, port_value: 10000 }
    filter_chains:
    - filters:
      - name: envoy.http_connection_manager
        config:
          access_log:
            name: envoy.file_access_log
            config:
              path: /tmp/envoy.log
              json_format:
                protocol: "%PROTOCOL%"
                duration: "%DURATION%"
                cluster: "%UPSTREAM_CLUSTER%"
                path: "%REQ(X-ENVOY-ORIGINAL-PATH?:PATH)%"
          stat_prefix: ingress_http
          codec_type: AUTO
          route_config:
            name: local_route
            virtual_hosts:
            - name: local_service
              domains: ["*"]
              routes:
              - match: { prefix: "/" }
                route: { host_rewrite: www.google.com, cluster: service_google }
          http_filters:
          - name: envoy.router
  clusters:
  - name: service_google
    connect_timeout: 0.25s
    type: LOGICAL_DNS
    # Comment out the following line to test on v6 networks
    dns_lookup_family: V4_ONLY
    lb_policy: ROUND_ROBIN
    hosts: [{ socket_address: { address: google.com, port_value: 443 }}]
    tls_context: { sni: www.google.com }

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM just two nits.

source/common/access_log/access_log_formatter.cc Outdated Show resolved Hide resolved
test/common/access_log/access_log_formatter_test.cc Outdated Show resolved Hide resolved
@htuch htuch self-assigned this Nov 13, 2018
Signed-off-by: Aaltan Ahmad <aa@stripe.com>
@aa-stripe
Copy link
Contributor Author

@htuch changes made!

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@htuch htuch merged commit cc991fe into envoyproxy:master Nov 14, 2018
fredlas pushed a commit to fredlas/envoy that referenced this pull request Mar 5, 2019
Tack a newline onto the end of all JSON access log lines

Risk Level: Low
Testing: Every JSON unit test will now check to see that there is one (and only one) newline in a log line
Docs Changes: None
Release Notes: None

Fixes envoyproxy#5018

Signed-off-by: Aaltan Ahmad <aa@stripe.com>
Signed-off-by: Fred Douglas <fredlas@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants