-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(new source):Initial addition of GCS support #23916
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
base: master
Are you sure you want to change the base?
feat(new source):Initial addition of GCS support #23916
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This draft looks great.
Is it possible to add an integration test?
Example:
counter!("gcp_cloud_storage_objects_filtered_total") | ||
.increment(1); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: add newline
// #[cfg(feature = "sources-gcp_cloud_storage")] | ||
// pub(crate) use self::gcp_cloud_storage::*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// #[cfg(feature = "sources-gcp_cloud_storage")] | |
// pub(crate) use self::gcp_cloud_storage::*; |
/// Automatically attempt to determine the compression scheme. | ||
/// | ||
/// The compression scheme of the object is determined from its `Content-Encoding` and | ||
/// `Content-Type` metadata, as well as the key suffix (for example, `.gz`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as well as the key suffix
let's explain a bit more what has precedence to avoid user confusion
src/sources/gcp_cloud_storage/mod.rs
Outdated
#[configurable(derived)] | ||
#[serde(default = "default_framing")] | ||
#[derivative(Default(value = "default_framing()"))] | ||
pub framing: FramingConfig, | ||
|
||
#[configurable(derived)] | ||
#[serde(default = "default_decoding")] | ||
#[derivative(Default(value = "default_decoding()"))] | ||
pub decoding: DeserializerConfig, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
#[configurable(derived)] | |
#[serde(default = "default_framing")] | |
#[derivative(Default(value = "default_framing()"))] | |
pub framing: FramingConfig, | |
#[configurable(derived)] | |
#[serde(default = "default_decoding")] | |
#[derivative(Default(value = "default_decoding()"))] | |
pub decoding: DeserializerConfig, | |
#[serde(flatten)] | |
pub encoding: EncodingConfigWithFraming, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isnt this strictly for sinks?
Co-authored-by: Pavlos Rontidis <[email protected]>
Just a note, ill be on leave for a little while, but will come back to this. I have a few of the suggestions implemented & cleaning up the code! |
DRAFT WHILE IN PROGRESS
Summary
This adds GCS as a source. It is built primarily off of the AWS S3 source concept (replacing SQS with pubsub, and S3 with GCS).
Vector configuration
How did you test this PR?
Change Type
Is this a breaking change?
Does this PR include user facing changes?
no-changelog
label to this PR.References
Closes: #7501
Notes
@vectordotdev/vector
to reach out to us regarding this PR.pre-push
hook, please see this template.make fmt
make check-clippy
(if there are failures it's possible some of them can be fixed withmake clippy-fix
)make test
git merge origin master
andgit push
.Cargo.lock
), pleaserun
make build-licenses
to regenerate the license inventory and commit the changes (if any). More details here.