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

Use httpRequest field rename #312

Merged
merged 12 commits into from
Jun 3, 2022
Merged

Use httpRequest field rename #312

merged 12 commits into from
Jun 3, 2022

Conversation

braydonk
Copy link
Contributor

@braydonk braydonk commented Jan 7, 2022

As specified in fluent/fluent-bit#4200 fluent-bit is using a name for logging.googleapis.com/httpRequest that is inconsistent (i.e. logging.googleapis.com/http_request). To allow for a non-breaking fix, this PR uses a feature added to fluent-bit that allows the user to override this key.

@braydonk braydonk requested review from a team and igorpeshansky and removed request for a team January 7, 2022 18:31
@qingling128
Copy link
Contributor

Looks like the Ngnix and Apache tests are failing:
image

BTW, looks like we don't have coverage for http request in ops_agent_test.go (internal). Can we add a test case there?

@braydonk braydonk force-pushed the braydonk-field-rename branch from 2c8486f to 9546d8a Compare February 7, 2022 14:53
Copy link
Contributor

@qingling128 qingling128 left a comment

Choose a reason for hiding this comment

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

LGTM modulo the PR is sync to HEAD

@braydonk
Copy link
Contributor Author

Thanks Ling, I'll clean this up once my change is released in fluent-bit upstream (likely in 1.8.13).

@braydonk braydonk force-pushed the braydonk-field-rename branch from 9546d8a to b8d03d1 Compare April 8, 2022 12:23
@braydonk braydonk force-pushed the braydonk-field-rename branch from b8d03d1 to 75472dd Compare May 30, 2022 17:56
@braydonk braydonk force-pushed the braydonk-field-rename branch from 75472dd to 9531df6 Compare May 31, 2022 20:54
@braydonk braydonk added the kokoro:force-run Forces kokoro to run integration tests on a CL label May 31, 2022
@stackdriver-instrumentation-release stackdriver-instrumentation-release removed the kokoro:force-run Forces kokoro to run integration tests on a CL label May 31, 2022
braydonk added 2 commits June 1, 2022 20:17
I think as a result of sloppy Git rebasing I left some new generated
goldens for an old deleted test in my last commit.
Add a new sub test case that will also check that the old field is not
parsed. Also added a check that the field is stripped from the message
as expected.
confgenerator/logging.go Show resolved Hide resolved
integration_test/ops_agent_test.go Outdated Show resolved Hide resolved
integration_test/ops_agent_test.go Outdated Show resolved Hide resolved
integration_test/ops_agent_test.go Outdated Show resolved Hide resolved
integration_test/ops_agent_test.go Outdated Show resolved Hide resolved
Changed wording of a comment and some variable names for clarity.
Changed log query to quote the log ID.
@braydonk
Copy link
Contributor Author

braydonk commented Jun 2, 2022

Will update this branch once I see these integration tests work.

Copy link
Member

@igorpeshansky igorpeshansky left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

integration_test/ops_agent_test.go Outdated Show resolved Hide resolved
@braydonk braydonk merged commit 9783472 into master Jun 3, 2022
@braydonk braydonk deleted the braydonk-field-rename branch June 9, 2022 12:46
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.

4 participants