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(internal_metrics source): Instrument few more components with metrics #2620

Merged
merged 9 commits into from
May 20, 2020

Conversation

Alexx-G
Copy link
Contributor

@Alexx-G Alexx-G commented May 16, 2020

This PR instruments few components with metrics following checklist from #2007
I've followed initial implementation from #1953

I have few open questions though:

  • Is there a convention for internal events naming? I've got a bit confused. E.g. ElasticSearchEventReceived defined for sink component, however most internal events define *Sent events for sinks and *Received events for sources. And since corresponding metric name in most places is events|bytes_processed, it's a bit confusing adding metrics for the first time.
  • Would it be possible to include component name as metric tag? Since same component type can be used multiple times in the config, it's super useful to be able to distinguish them.

Signed-off-by: Alex Gavrisco <alexandr@gavrisco.com>
Signed-off-by: Alex Gavrisco <alexandr@gavrisco.com>
Signed-off-by: Alex Gavrisco <alexandr@gavrisco.com>
Signed-off-by: Alex Gavrisco <alexandr@gavrisco.com>
Signed-off-by: Alex Gavrisco <alexandr@gavrisco.com>
Signed-off-by: Alex Gavrisco <alexandr@gavrisco.com>
Signed-off-by: Alex Gavrisco <alexandr@gavrisco.com>
Signed-off-by: Alex Gavrisco <alexandr@gavrisco.com>
Signed-off-by: Alex Gavrisco <alexandr@gavrisco.com>
@Alexx-G Alexx-G changed the title enhancement(internal_metrics source): Instrument few more components with metrics feat(internal_metrics source): Instrument few more components with metrics May 16, 2020
Copy link
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.

These look great, thanks so much @Alexx-G!

@lukesteensen lukesteensen merged commit 741690a into vectordotdev:master May 20, 2020
@binarylogic
Copy link
Contributor

Thanks @Alexx-G!

@Alexx-G
Copy link
Contributor Author

Alexx-G commented May 21, 2020

Thanks for reviewing it @lukesteensen !
@binarylogic @lukesteensen Would it be possible to include component name as metric tag? It would slightly increase metrics volume, on the other hand it would allow distinguishing metrics for same component used multiple times (e.g. it's possible to flush data to 3 different Kafka sinks with different SLA - e.g. self provisioned kafka vs MSK, thus aggregated view doesn't tell which one fails more often).

@Alexx-G Alexx-G deleted the add-metrics branch May 21, 2020 10:20
@binarylogic
Copy link
Contributor

@Alexx-G I've opened #2660 for that. It's a caveat we noted during the initial implementation and hope to solve in the future. Until then, we can attempt to manually pass this data to the event. I'm not sure if that's possible. Ideally, we'd carry over the tracing context as noted in that issue.

mengesb pushed a commit to jacobbraaten/vector that referenced this pull request Dec 9, 2020
…trics (vectordotdev#2620)

Signed-off-by: Brian Menges <brian.menges@anaplan.com>
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.

None yet

3 participants