Skip to content

Conversation

@benjamin-awd
Copy link
Contributor

@benjamin-awd benjamin-awd commented Mar 21, 2025

Summary

This PR adds support for VRL in query parameters within the http client source.

This is useful for making GET requests with dynamic parameters e.g. a start_time parameter which can help to filter extraneous data (e.g. only get "active" events based on the current timestamp, or events from the last hour)

e.g.

sources:
  http:
    type: http_client
    endpoint: https://endpoint.com
    method: GET
    query:
      timestamp:
        type: vrl
        value: 'to_unix_timestamp(now(), unit: "milliseconds")'

Which will resolve to https://endpoint.com?timestamp=1749291799121
Closes #14621, #22988

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?

Tests have been added, but otherwise we've been running this in a Kubernetes environment with Helm.

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.
    • make check-all is a good command to run locally. This check is
      defined here. Some of these
      checks might not be relevant to your PR. For Rust changes, at the very least you should run:
      • cargo fmt --all
      • cargo clippy --workspace --all-targets -- -D warnings
      • cargo nextest run --workspace (alternatively, you can run cargo test --all)

References

@benjamin-awd benjamin-awd requested a review from a team as a code owner March 21, 2025 16:32
@github-actions github-actions bot added the domain: sources Anything related to the Vector's sources label Mar 21, 2025
@benjamin-awd benjamin-awd marked this pull request as draft March 27, 2025 02:09
@benjamin-awd benjamin-awd force-pushed the http-client-source-vrl branch from cb79433 to 834d166 Compare March 27, 2025 14:56
@benjamin-awd benjamin-awd marked this pull request as ready for review March 27, 2025 14:58
@benjamin-awd benjamin-awd force-pushed the http-client-source-vrl branch 3 times, most recently from ff535bb to 4407fff Compare March 27, 2025 15:52
@benjamin-awd benjamin-awd requested a review from a team as a code owner April 16, 2025 15:32
@github-actions github-actions bot added the domain: external docs Anything related to Vector's external, public documentation label Apr 16, 2025
@pront pront requested a review from Copilot April 16, 2025 15:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • website/cue/reference/components/sources/base/http_client.cue: Language not supported
Comments suppressed due to low confidence (3)

src/sources/http_client/client.rs:398

  • [nitpick] Consider renaming 'process_query_value' to 'evaluate_query_parameter' to more clearly indicate that the function evaluates dynamic VRL expressions.
fn process_query_value(value: &QueryParameterValue) -> QueryParameterValue {

src/sources/http_client/client.rs:241

  • If VRL expression compilation fails, the warning is logged and the parameter is silently returned unevaluated. Consider whether it is preferable to fallback to the original input or signal a compilation failure to prevent unexpected behavior.
if let Ok(value) = Runtime::default().resolve(&mut target, &program, &timezone) {

src/sources/http_client/tests.rs:231

  • Consider adding a test case for scenarios where VRL expression compilation fails to ensure the system gracefully handles such errors.
/// VRL query parameters should be parsed correctly

@pront pront added this pull request to the merge queue Apr 28, 2025
@pront pront removed this pull request from the merge queue due to a manual request Apr 28, 2025
@pront pront added the source: http_client Anything `http_client` source related label May 1, 2025
@pront pront added the meta: awaiting author Pull requests that are awaiting their author. label May 12, 2025
@github-actions github-actions bot added the domain: sinks Anything related to the Vector's sinks label Jun 7, 2025
@benjamin-awd benjamin-awd force-pushed the http-client-source-vrl branch from d07ca7f to 83fbf80 Compare June 7, 2025 10:10
@benjamin-awd benjamin-awd force-pushed the http-client-source-vrl branch 2 times, most recently from c402043 to e753fac Compare June 7, 2025 10:29
@benjamin-awd benjamin-awd force-pushed the http-client-source-vrl branch from e753fac to 637d427 Compare June 7, 2025 12:47
Copy link
Member

@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.

A few small comments. But LGTM!

src/http.rs Outdated
Comment on lines 606 to 608
rename = "type"
)]
param_type: ParamType,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
rename = "type"
)]
param_type: ParamType,
)]
r#type: ParamType,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes made in 37952e8

@benjamin-awd
Copy link
Contributor Author

Hm I see that the CI is failing here:

error: this could be a `const fn`
Error:    --> src/http.rs:622:5
    |
622 | /     pub fn value(&self) -> &str {
623 | |         match self {
624 | |             ParameterValue::String(s) => s,
625 | |             ParameterValue::Typed { value, .. } => value,
626 | |         }
627 | |     }
    | |_____^

but I'm not sure in this case if it can actually be a const function

@pront pront removed the meta: awaiting author Pull requests that are awaiting their author. label Jun 17, 2025
@pront
Copy link
Member

pront commented Jun 17, 2025

Hm I see that the CI is failing here:

error: this could be a `const fn`
Error:    --> src/http.rs:622:5
    |
622 | /     pub fn value(&self) -> &str {
623 | |         match self {
624 | |             ParameterValue::String(s) => s,
625 | |             ParameterValue::Typed { value, .. } => value,
626 | |         }
627 | |     }
    | |_____^

but I'm not sure in this case if it can actually be a const function

Could be a false positive. You can add an allow block to exclude it from this check.

@pront pront enabled auto-merge June 17, 2025 16:22
@pront pront added this pull request to the merge queue Jun 17, 2025
Merged via the queue into vectordotdev:master with commit 05d1521 Jun 17, 2025
42 checks passed
@pront
Copy link
Member

pront commented Jun 17, 2025

Thanks, this is a useful feature!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: external docs Anything related to Vector's external, public documentation domain: sinks Anything related to the Vector's sinks domain: sources Anything related to the Vector's sources source: http_client Anything `http_client` source related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Templating query arguments for http_scrape

4 participants