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

Impl Logger AddContext summand #162

Merged
merged 40 commits into from
Jan 6, 2020
Merged

Impl Logger AddContext summand #162

merged 40 commits into from
Jan 6, 2020

Conversation

IvantheTricourne
Copy link
Contributor

@IvantheTricourne IvantheTricourne commented Dec 23, 2019

Closes #161.

hs-abci-sdk/src/Tendermint/SDK/BaseApp/Logger.hs Outdated Show resolved Hide resolved
@@ -1,23 +1,147 @@
# nameservice

## Metrics
## Metrics via Prometheus
Copy link
Collaborator

Choose a reason for hiding this comment

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

Before commenting on any of this, is there any way to have kibana pull metrics from prometheus in the same way datadog does. This would be good to fix some confusing asymmetry between server metrics and application metrics, and would also be easier to explain that you can now use either Kibana or datadog to view anything you want, or both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I’ll investigate.

Copy link
Contributor Author

@IvantheTricourne IvantheTricourne Dec 24, 2019

Choose a reason for hiding this comment

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

@blinky3713

It seems MetricBeat is an option from the ELK folks that can scrape prometheus exporters and servers. Given we're already using ELK stack, this seems to be a pretty good option.

Similar to the datadog agent, it also has a docker-compose.

Copy link
Collaborator

@martyall martyall left a comment

Choose a reason for hiding this comment

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

There is a problem with checkTx / deliverTx with how this is now. Namely, if checkTx is actually running transactions without committing state, then it's going to log the same event to ES twice. We need to set a context for check versus deliver, and probably need to set context for the transaction hash as well so things are easily searchable.

I think it may make more sense to leave this unsolved here and instead fix it in #167 since transactions are further isolated in that PR

hs-abci-sdk/src/Tendermint/SDK/BaseApp/Events.hs Outdated Show resolved Hide resolved
@martyall martyall merged commit 1101bd1 into master Jan 6, 2020
@martyall martyall deleted the context-summand branch January 6, 2020 17:20
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.

Add logger context summand
2 participants