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

enhancement: allow more missing upstream info in parse_nginx_log #498

Merged
merged 3 commits into from
Oct 10, 2023

Conversation

ex5
Copy link
Contributor

@ex5 ex5 commented Oct 9, 2023

There's a valid use case when upstream_response_length, upstream_response_time, upstream_status are sometimes unavailable but the rest of the log message still matches the upstreaminfo format (e.g. static files served by the same virtual host). Here's an example of such a message:

1.1.1.1 - - [09/Oct/2023:10:49:43 +0200] "GET /media/cache/f4/b4/f4b494deadbeefc8.jpg HTTP/1.1" 200 32235 "https://example.com/page/" "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/117.0.0.0 Safari/537.36" 951 0.000 [upstream-production-service-1234] [upstream-production-service-1234] - - - - 7d1f3deadbeefcbb3bc

Currently, this kind of message generates a parsing error and some logging data is lost. It would be useful if instead the rest of the available data was still parsed correctly.

Copy link
Collaborator

@pront pront left a comment

Choose a reason for hiding this comment

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

Hi @ex5, this change seems reasonable to me, thanks!

Two things we still need:

  • Add a changelog.md entry for this change
  • Update the Vector docs here with a new example with arguments like the ingress_nginx_upstreaminfo_valid_missing_upstream.
    • cue fmt path/to/file and make check-component-docs are the commands you will need.

Copy link
Collaborator

@pront pront left a comment

Choose a reason for hiding this comment

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

Looks good, thank you.

@pront pront added this pull request to the merge queue Oct 10, 2023
Merged via the queue into vectordotdev:main with commit a8469ad Oct 10, 2023
9 checks passed
@ex5 ex5 deleted the upstreaminfo-allow-more-missing branch October 11, 2023 14:13
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