Skip to content

Plumb BoolIndicator through stats sink and admin#6279

Closed
taion809 wants to merge 3 commits intoenvoyproxy:masterfrom
taion809:bugifx/GH-6277
Closed

Plumb BoolIndicator through stats sink and admin#6279
taion809 wants to merge 3 commits intoenvoyproxy:masterfrom
taion809:bugifx/GH-6277

Conversation

@taion809
Copy link
Contributor

@taion809 taion809 commented Mar 13, 2019

WIP
Description: Plumb the boolIndicators through various stats sinks and handlers. This also needs to cast from bool to int for collectors like statsd and prometheus.
Risk Level: Low
Testing: Manual testing

Fixes: #6277

Signed-off-by: Nicholas J nicholas.a.johns5@gmail.com

Fixes: envoyproxy#6277

Signed-off-by: Nicholas J <nicholas.a.johns5@gmail.com>
Signed-off-by: Nicholas J <nicholas.a.johns5@gmail.com>
@mattklein123
Copy link
Member

@fredlas can you do an initial pass on this and/or pair with @taion809? I don't want to skimp on tests here, so if it's easier to just revert your other PR for now we can also do that. Let us know what you think.

@taion809
Copy link
Contributor Author

SGTM, i'm currently working on tests right but but I agree if it's easier to revert we should do that so prod deployments dont get blocked

@mattklein123
Copy link
Member

@taion809 I think my feeling is to revert the other change, and then @fredlas can fix at a more convenient pace? WDYT?

@mattklein123
Copy link
Member

#6280

@taion809
Copy link
Contributor Author

taion809 commented Mar 13, 2019

SGTM, one thing to consider though is that when outputting the metrics statsd and prometheus both have no boolean datatype, they need to be converted to an int and output as a gauge :)

@taion809 taion809 closed this Mar 13, 2019
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.

3 participants