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

feat(config): Global Default Log Schemas #1769

Merged
merged 18 commits into from Feb 14, 2020

Conversation

@Hoverbear
Copy link
Contributor

Hoverbear commented Feb 10, 2020

This addresses #1446.

It adds a log_schema field to the config.global. It goes into config.global since this already gets passed to the SourceConfig::build event. It also exists in the $SOURCE_source function args as this is used in some tests.

Example:

[hoverbear@obsidian:~/git/vector]$ cat test.toml 
data_dir = '/var/lib/vector/'
dns_servers = []

[log_schema]
host_key = "instance" # default "host"
message_key = "info" # default "message"
timestamp_key = "datetime" # default "timestamp"

[sources.source0]
max_length = 102400
type = 'stdin'
[sinks.sink0]
healthcheck = true
inputs = ['source0']
type = 'console'
encoding = "json"

[sinks.sink0.buffer]
type = 'memory'
max_events = 500
when_full = 'block'


[hoverbear@obsidian:~/git/vector]$ cargo run -- --config test.toml -qqq
# ....
Feb 13 04:24:30.902  INFO vector::topology: Starting sink "sink0"
Feb 13 04:24:30.902  INFO source{name=source0 type=stdin}: vector::sources::stdin: Capturing STDIN
asd
{"info":"asd","datetime":"2020-02-13T12:24:32.632275500Z","instance":"OBSIDIAN"}
@Hoverbear Hoverbear added this to the Initial schema support milestone Feb 10, 2020
@Hoverbear Hoverbear self-assigned this Feb 10, 2020
@Hoverbear Hoverbear changed the title [Draft] Global Default Log Schemas feature(config): Global Default Log Schemas [Draft] Feb 10, 2020
@binarylogic

This comment has been minimized.

Copy link
Member

binarylogic commented Feb 10, 2020

The config syntax looks good! But shouldn't the resulting payload in your ouput have both the datetime and info keys since you didn't explicitly supply those as part of your input?

@Hoverbear

This comment has been minimized.

Copy link
Contributor Author

Hoverbear commented Feb 11, 2020

Ok! That clarifies it for me! I wasn't really sure if only the fields the source had configs for would be affected, or all would.

Copy link
Member

bruceg left a comment

I have left a few questions inline.

AFAICT the kafka sink also appears to use these keys. Should it also be modified to use the new config?

src/sources/file.rs Outdated Show resolved Hide resolved
src/sources/stdin.rs Outdated Show resolved Hide resolved
src/sources/stdin.rs Outdated Show resolved Hide resolved
src/topology/config/mod.rs Outdated Show resolved Hide resolved
src/topology/config/mod.rs Outdated Show resolved Hide resolved
src/topology/config/mod.rs Outdated Show resolved Hide resolved
@Hoverbear

This comment has been minimized.

Copy link
Contributor Author

Hoverbear commented Feb 11, 2020

Thanks for this early review @bruceg :)

@Hoverbear Hoverbear requested review from a-rodin and MOZGIII as code owners Feb 12, 2020
@binarylogic binarylogic requested review from bruceg and removed request for MOZGIII, lukesteensen, a-rodin and LucioFranco Feb 12, 2020
@binarylogic binarylogic assigned bruceg and unassigned Hoverbear Feb 12, 2020
Signed-off-by: Ana Hobden <operator@hoverbear.org>
@Hoverbear Hoverbear force-pushed the Hoverbear:templating-add-field branch from de54b42 to 1944719 Feb 12, 2020
Hoverbear added 6 commits Feb 12, 2020
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Signed-off-by: Ana Hobden <operator@hoverbear.org>
@Hoverbear Hoverbear requested a review from Jeffail as a code owner Feb 13, 2020
@Hoverbear Hoverbear changed the title feature(config): Global Default Log Schemas [Draft] feature(config): Global Default Log Schemas Feb 13, 2020
@Hoverbear Hoverbear changed the title feature(config): Global Default Log Schemas feat(config): Global Default Log Schemas Feb 13, 2020
Hoverbear added 3 commits Feb 13, 2020
fmt
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Copy link
Member

Jeffail left a comment

LGTM, but need to initialize the globals before running unit tests as well.

Hoverbear added 3 commits Feb 13, 2020
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Signed-off-by: Ana Hobden <operator@hoverbear.org>
@Hoverbear

This comment has been minimized.

Copy link
Contributor Author

Hoverbear commented Feb 13, 2020

@Jeffail I did it the easy way. :)

// TODO: Help Rust project support before_each
// Support uninitialized schemas in tests to help our contributors.
// Don't do it in release because that is scary.
#[cfg(debug_assertions)]

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Feb 13, 2020

Author Contributor

I am not 100% sure on this.

src/event/mod.rs Outdated Show resolved Hide resolved
src/event/mod.rs Outdated Show resolved Hide resolved
@@ -332,6 +335,35 @@ impl Config {
self.global.dns_servers.sort();
self.global.dns_servers.dedup();

// If the user has multiple config files, we must *merge* log schemas until we meet a
// conflict, then we are allowed to error.
let default_schema = event::LogSchema::default();

This comment has been minimized.

Copy link
@LucioFranco

LucioFranco Feb 13, 2020

Member

nit: prob could just do if with.global.log_schema = LogSchema::default()

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Feb 13, 2020

Author Contributor

We use it later though :o

src/event/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

LucioFranco left a comment

Overall this looks fantastic, turned out much better than I thought it would :)

Signed-off-by: Ana Hobden <operator@hoverbear.org>
@@ -143,6 +143,7 @@ evmap = { version = "7", features = ["bytes"] }
logfmt = "0.0.2"
notify = "4.0.14"
once_cell = "1.3"
getset = "0.1.0"

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Feb 13, 2020

Author Contributor

When you get to import your own library!

@Hoverbear

This comment has been minimized.

Copy link
Contributor Author

Hoverbear commented Feb 13, 2020

@LucioFranco Take a look again?

Hoverbear added 2 commits Feb 13, 2020
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Signed-off-by: Ana Hobden <operator@hoverbear.org>
@@ -434,6 +437,7 @@ mod test {
}

#[test]
#[ignore]

This comment has been minimized.

Copy link
@LucioFranco

LucioFranco Feb 13, 2020

Member

why are these ignored?

This comment has been minimized.

Copy link
@Hoverbear

Hoverbear Feb 13, 2020

Author Contributor

Indeed why!

Copy link
Member

LucioFranco left a comment

Just have one question about those ignored tests otherwise LGTM

Hoverbear added 2 commits Feb 13, 2020
Signed-off-by: Ana Hobden <operator@hoverbear.org>
Signed-off-by: Ana Hobden <operator@hoverbear.org>
@Hoverbear Hoverbear added this to In progress in Weekly Sprint (Feb 10, 2020) via automation Feb 14, 2020
@Hoverbear Hoverbear merged commit 15e69c8 into timberio:master Feb 14, 2020
13 checks passed
13 checks passed
cargo-deny
Details
Header rules Deploy canceled
Details
Mixed content Deploy canceled
Details
Pages changed Deploy canceled
Details
Redirect rules Deploy canceled
Details
DCO DCO
Details
Semantic Pull Request ready to be squashed
Details
ci/circleci: check-blog Your tests passed on CircleCI!
Details
ci/circleci: check-code Your tests passed on CircleCI!
Details
ci/circleci: check-fmt Your tests passed on CircleCI!
Details
ci/circleci: check-generate Your tests passed on CircleCI!
Details
ci/circleci: test-stable Your tests passed on CircleCI!
Details
deploy/netlify Deploy preview canceled.
Details
Weekly Sprint (Feb 10, 2020) automation moved this from In progress to Done Feb 14, 2020
@binarylogic

This comment has been minimized.

Copy link
Member

binarylogic commented Feb 14, 2020

Nice work on this! I just wanted to couple doc changes with it. I'll follow up with a doc change PR.

bednar added a commit to bonitoo-io/vector that referenced this pull request Feb 17, 2020
* Global log schemas

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Tests working and some lints cleaned.

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Fix unix tests

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Fixup of remaining unix tests

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Use correct naming.

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Add better documentation for handling conflicting log_schemas.

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* fmt

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Fixup schema lookup in logdna

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Fix benches

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Fmt and fix lints

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Fix some tests that are dead on Windows.

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Docker nits

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Fix naming

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Fix a test

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Missed one!

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Unignore tests

Signed-off-by: Ana Hobden <operator@hoverbear.org>

* Reduce visiblility!

Signed-off-by: Ana Hobden <operator@hoverbear.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.