fix(kubernetes_logs source): Set user-agent for k8s apiserver requests. #21905

ganelo · 2024-11-27T18:58:41Z

Summary

When querying the k8s api server for pods, the client created by the kubernetes_log source failed to specify a user agent, making it difficult to correctly attribute load on the api server to vector. By bumping the version of the kube and k8s-openapi crates, we get access to sending arbitrary headers with our requests. This PR uses this functionality to set a user-agent header to the value vector/<vector version>.

Change Type

Bug fix
New feature
Non-functional (chore, refactoring, docs)
Performance

Is this a breaking change?

Yes
No

How did you test this PR?

I built vector from source on this branch and deployed it via a helm chart in a k8s cluster to confirm that queries against the k8s api server were being generated with user agent set as intended. I also confirmed that logs from the kubernetes_log source continue to be processed as expected.

Does this PR include user facing changes?

Yes. Please add a changelog fragment based on our guidelines.
No. A maintainer will apply the "no-changelog" label to this PR.

Checklist

Please read our Vector contributor resources.
If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
run dd-rust-license-tool write to regenerate the license inventory and commit the changes (if any). More details here.

References

bits-bot · 2024-11-27T18:58:46Z

All committers have signed the CLA.

jszwedko

Thank you! This looks good to me. I have just one question about the deprecation allows.

src/aws/auth.rs

src/sources/kubernetes_logs/mod.rs

changelog.d/21864_kubernetes_logs_user_agent.fix.md

pront · 2024-12-05T16:47:03Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

Also, please take a look at this.

Co-authored-by: Pavlos Rontidis <[email protected]>

…vector into og/kubernetes-user-agent

jszwedko

Thanks @ganelo !

jszwedko

@ganelo It looks like the AWS integration tests are failing here. You can run these locally via make test-integration-aws (requires Docker). I think it is likely due to the bump to aws-config since we've been trying to bump it separately in #20663 without success (also failing integration tests).

If it is possible to bump aws-config back down, that might unblock this PR.

ganelo · 2024-12-16T20:49:15Z

@ganelo It looks like the AWS integration tests are failing here. You can run these locally via make test-integration-aws (requires Docker). I think it is likely due to the bump to aws-config since we've been trying to bump it separately in #20663 without success (also failing integration tests).

If it is possible to bump aws-config back down, that might unblock this PR.

@jszwedko It looks like all relevant tests are passing now - mind merging?

jszwedko · 2024-12-16T21:03:57Z

Cargo.toml

-aws-sdk-firehose = { version = "1.3.0", default-features = false, features = ["behavior-version-latest", "rt-tokio"], optional = true }
-aws-sdk-kinesis = { version = "1.3.0", default-features = false, features = ["behavior-version-latest", "rt-tokio"], optional = true }
-aws-sdk-secretsmanager = { version = "1.3.0", default-features = false, features = ["behavior-version-latest", "rt-tokio"], optional = true }
+aws-sdk-sqs = { version = "~1.3.0", default-features = false, features = ["behavior-version-latest", "rt-tokio"], optional = true }


I'm unsure if we need all of these ~s rather than just not updating aws-config when adding the new crates. Did you find them to be required?

Truthfully I was having a hard time running the test locally (was seeing the same result of test failures whether running on tip of master or on this branch) so I may have been a little overzealous here. I'd be happy to try just aws-config if you don't mind triggering the integration test here to check.

👍 the integration tests will run during the merge so we should get feedback then. Unfortunately it isn't possible to trigger them on external PRs otherwise at the moment.

Gotcha. Okay I'll go ahead and revert the extra version pins and echo back.

…s. (#21905) * fix(kubernetes_log source): Set user-agent for k8s apiserver requests. * Update changelog.d/21864_kubernetes_logs_user_agent.fix.md Co-authored-by: Pavlos Rontidis <[email protected]> * Switch away from deprecated structs * Update 21864_kubernetes_logs_user_agent.fix.md * Try manually updating version via crate update * Specify using tilde in Cargo.toml * Rip out unused aws-runtime dep * Try to limit dep bumps * Revert superfluous import change * Remove superfluous pins --------- Co-authored-by: Orri Ganel <[email protected]> Co-authored-by: Pavlos Rontidis <[email protected]>

ganelo · 2024-12-17T18:38:35Z

@jszwedko I added features = ["std"] to try to fix the http-1 compilation errors from the merge jobs, not really sure what the issue is with awscli not being found.

jszwedko · 2024-12-17T19:15:26Z

not really sure what the issue is with awscli not being found.

That is actually fixed by #22044 . Let me try to merge this again.

ganelo · 2024-12-18T15:29:29Z

@jszwedko so it looks like test-cli was cancelled (probably due to the next issue) and make in K8s v1.23.3 / docker (essential) is failing with error 101 which I have to believe is unrelated to this PR (possibly an OOM?)

jszwedko · 2024-12-18T16:17:45Z

Thanks for flagging @ganelo . I think the CLI test timeout needs to be bumped so I opened #22056

For the k8s tests I see at least on failure:

thread 'simple_checkpoint' panicked at lib/k8s-e2e-tests/tests/vector-agent.rs:2105:9:
assertion `left == right` failed
  left: String("MARKER")
 right: "CHECKED_MARKER"

This might be a race condition though. I'll retry it once the CLI test timeout is merged.

ganelo · 2024-12-18T18:49:42Z

Thanks for the update @jszwedko - let me know if the retry fails; the only thing I could think of would be to wait to process InitApply events until we get an InitDone rather than processing them as they come in, but I'm not sure that quite makes sense as being causally related here.

ganelo · 2024-12-18T22:10:01Z

@jszwedko Thanks for all the assistance in getting this over the finish line, and sorry for the back and forth!

jszwedko · 2024-12-18T22:15:13Z

@jszwedko Thanks for all the assistance in getting this over the finish line, and sorry for the back and forth!

Thank you for implementing the feature!

ganelo requested a review from a team as a code owner November 27, 2024 18:58

github-actions bot added the domain: sources Anything related to the Vector's sources label Nov 27, 2024

ganelo mentioned this pull request Nov 27, 2024

kubernetes_log source: Specify a user agent when making requests to the k8s api server #21864

Closed

fix(kubernetes_log source): Set user-agent for k8s apiserver requests.

735e4c9

ganelo force-pushed the og/kubernetes-user-agent branch from b0cc62f to 735e4c9 Compare November 27, 2024 20:14

jszwedko reviewed Dec 5, 2024

View reviewed changes

src/aws/auth.rs Outdated Show resolved Hide resolved

src/sources/kubernetes_logs/mod.rs Show resolved Hide resolved

pront reviewed Dec 5, 2024

View reviewed changes

changelog.d/21864_kubernetes_logs_user_agent.fix.md Outdated Show resolved Hide resolved

pront approved these changes Dec 5, 2024

View reviewed changes

ganelo and others added 5 commits December 6, 2024 13:42

Update changelog.d/21864_kubernetes_logs_user_agent.fix.md

c3cc343

Co-authored-by: Pavlos Rontidis <[email protected]>

Switch away from deprecated structs

4ac20b3

Merge branch 'og/kubernetes-user-agent' of https://github.com/ganelo/…

2540c7a

…vector into og/kubernetes-user-agent

Merge branch 'master' into og/kubernetes-user-agent

4c71990

Merge branch 'master' into og/kubernetes-user-agent

bbe73df

jszwedko approved these changes Dec 6, 2024

View reviewed changes

jszwedko enabled auto-merge December 6, 2024 21:04

ganelo changed the title ~~fix(kubernetes_log source): Set user-agent for k8s apiserver requests.~~ fix(kubernetes_logs source): Set user-agent for k8s apiserver requests. Dec 6, 2024

Update 21864_kubernetes_logs_user_agent.fix.md

3d63059

auto-merge was automatically disabled December 6, 2024 21:28
Head branch was pushed to by a user without write access

pront enabled auto-merge December 6, 2024 22:00

jszwedko reviewed Dec 9, 2024

View reviewed changes

Try manually updating version via crate update

2804e98

auto-merge was automatically disabled December 10, 2024 18:48
Head branch was pushed to by a user without write access

Specify using tilde in Cargo.toml

c6f082b

jszwedko enabled auto-merge December 11, 2024 09:54

Rip out unused aws-runtime dep

d504bad

auto-merge was automatically disabled December 13, 2024 17:02
Head branch was pushed to by a user without write access

oganel added 2 commits December 13, 2024 12:08

Try to limit dep bumps

de8e5fc

Revert superfluous import change

fab73f1

jszwedko reviewed Dec 16, 2024

View reviewed changes

Remove superfluous pins

cc4196f

jszwedko approved these changes Dec 16, 2024

View reviewed changes

jszwedko enabled auto-merge December 16, 2024 23:14

jszwedko added this pull request to the merge queue Dec 17, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 17, 2024

Try to fix compilation error in merge jobs

addc91f

jszwedko enabled auto-merge December 17, 2024 19:15

jszwedko added this pull request to the merge queue Dec 17, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 17, 2024

jszwedko added this pull request to the merge queue Dec 18, 2024

Merged via the queue into vectordotdev:master with commit 434e742 Dec 18, 2024
41 checks passed

ganelo deleted the og/kubernetes-user-agent branch December 19, 2024 16:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(kubernetes_logs source): Set user-agent for k8s apiserver requests. #21905

fix(kubernetes_logs source): Set user-agent for k8s apiserver requests. #21905

ganelo commented Nov 27, 2024

bits-bot commented Nov 27, 2024 •

edited

Loading

jszwedko left a comment

pront commented Dec 5, 2024

jszwedko left a comment

jszwedko left a comment

ganelo commented Dec 16, 2024 •

edited

Loading

jszwedko Dec 16, 2024

ganelo Dec 16, 2024

jszwedko Dec 16, 2024

ganelo Dec 16, 2024

ganelo commented Dec 17, 2024

jszwedko commented Dec 17, 2024

ganelo commented Dec 18, 2024

jszwedko commented Dec 18, 2024

ganelo commented Dec 18, 2024

ganelo commented Dec 18, 2024

jszwedko commented Dec 18, 2024

fix(kubernetes_logs source): Set user-agent for k8s apiserver requests. #21905

fix(kubernetes_logs source): Set user-agent for k8s apiserver requests. #21905

Conversation

ganelo commented Nov 27, 2024

Summary

Change Type

Is this a breaking change?

How did you test this PR?

Does this PR include user facing changes?

Checklist

References

bits-bot commented Nov 27, 2024 • edited Loading

jszwedko left a comment

Choose a reason for hiding this comment

pront commented Dec 5, 2024

jszwedko left a comment

Choose a reason for hiding this comment

jszwedko left a comment

Choose a reason for hiding this comment

ganelo commented Dec 16, 2024 • edited Loading

jszwedko Dec 16, 2024

Choose a reason for hiding this comment

ganelo Dec 16, 2024

Choose a reason for hiding this comment

jszwedko Dec 16, 2024

Choose a reason for hiding this comment

ganelo Dec 16, 2024

Choose a reason for hiding this comment

ganelo commented Dec 17, 2024

jszwedko commented Dec 17, 2024

ganelo commented Dec 18, 2024

jszwedko commented Dec 18, 2024

ganelo commented Dec 18, 2024

ganelo commented Dec 18, 2024

jszwedko commented Dec 18, 2024

bits-bot commented Nov 27, 2024 •

edited

Loading

ganelo commented Dec 16, 2024 •

edited

Loading