Skip to content

feat(blackhole sink): accept metric events, too#1237

Merged
loony-bean merged 1 commit intovectordotdev:masterfrom
thoughtpolice:feature/blackhole-metrics
Nov 23, 2019
Merged

feat(blackhole sink): accept metric events, too#1237
loony-bean merged 1 commit intovectordotdev:masterfrom
thoughtpolice:feature/blackhole-metrics

Conversation

@thoughtpolice
Copy link
Copy Markdown
Contributor

The Vector website indicates that the blackhole sink accepts both log
and metric events, but this isn't true: it only accepts logs. There's no
reason this should be the case.

In particular, for metric sources (such as statsd in my case) that
want to accept/transform metrics but just send everything to /dev/null
for the moment (e.g. when debugging your configuration), there's no easy
way to do this, resulting in an annoying error in the startup topology
check.

I also like enabling blackholes with high print_amount settings
anyway, so that my journald logs show some progress occasionally. This
will allow me to send all streams to one blackhole that prints that
info.

This patch simply takes events and distinguish metrics, adding them to
the total event count, and taking the total length of the .to_string()
method for the byte size, AKA the json representation. I believe this is
roughly the right metric, since it is basically a measure of "how many
bytes this stream will roughly push out", which will generally be some
rendered form of the metric data -- such as JSON.

@binarylogic
Copy link
Copy Markdown
Contributor

Thanks @thoughtpolice! We'll get this reviewed.

@loony-bean do you mind reviewing this? Thanks.

Copy link
Copy Markdown
Member

@lukesteensen lukesteensen left a comment

Choose a reason for hiding this comment

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

Thanks for this! We still have some work to do with the Any type for sources and transforms, but we should definitely be using it here.

@lukesteensen
Copy link
Copy Markdown
Member

@thoughtpolice Would you mind running cargo fmt and pushing the changes? Would be happy to merge once that check is green.

The Vector website indicates that the blackhole sink accepts both log
and metric events, but this isn't true: it only accepts logs. There's no
reason this should be the case.

In particular, for metric sources (such as `statsd` in my case) that
want to accept/transform metrics but just send everything to `/dev/null`
for the moment (e.g. when debugging your configuration), there's no easy
way to do this, resulting in an annoying error in the startup topology
check.

I also like enabling blackholes with high `print_amount` settings
anyway, so that my `journald` logs show some progress occasionally. This
will allow me to send all streams to one blackhole that prints that
info.

This patch simply takes events and distinguish metrics, adding them to
the total event count, and taking the total length of the .to_string()
method for the byte size, AKA the json representation. I believe this is
roughly the right metric, since it is basically a measure of "how many
bytes this stream will roughly push out", which will generally be some
rendered form of the metric data -- such as JSON.

Signed-off-by: Austin Seipp <aseipp@pobox.com>
@thoughtpolice thoughtpolice force-pushed the feature/blackhole-metrics branch from a7175c8 to 17d90b9 Compare November 23, 2019 06:15
@loony-bean loony-bean merged commit 52a49d5 into vectordotdev:master Nov 23, 2019
@thoughtpolice thoughtpolice deleted the feature/blackhole-metrics branch December 3, 2019 20:30
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