Skip to content

Conversation

koddsson
Copy link
Member

Fixes #245!

Collects metrics and makes 'em available on port 8091 under /metrics.

@MiniGod
Copy link
Member

MiniGod commented Feb 15, 2016

Nice!
I assume these metrics are reset after restarting the application?

@koddsson
Copy link
Member Author

Oh, yeah. I didn't think of that 😁

@koddsson
Copy link
Member Author

Maybe it's better to send 'em to a statsd server since the lib has support for that. I'll check if we can do that easily.

@kristjanmik
Copy link
Member

It is probably better to did this in front of the application layer itself. I was taking a look at https://getkong.org the other day and it seems like the perfect fit for us. Then all this logging comes for free.

@koddsson
Copy link
Member Author

@kristjanmik If we do something like kong, we'd just have to add it to our infrastructure?

@koddsson
Copy link
Member Author

In any case should we merge this for now and then iterate on it? There's instant value to be had even if we lose the data once we restart the app and if it proves valuable we can go with either something like kong or start shipping the data to some server for safe keeping.

@koddsson
Copy link
Member Author

Also kong looks really nice! 🐵

@kristjanmik
Copy link
Member

I agree, I'm merging this in!

kristjanmik added a commit that referenced this pull request Feb 15, 2016
@kristjanmik kristjanmik merged commit 461cd3e into master Feb 15, 2016
@kristjanmik kristjanmik deleted the add-metrics-endpoint branch February 15, 2016 19:07
@MiniGod
Copy link
Member

MiniGod commented Feb 15, 2016

I assume the port is not exposed? Maybe we should just add it to the nginx config as a path?
I dislike loading pages on random ports. i.e. expose it on apis.is/metrics or something?

@koddsson
Copy link
Member Author

Where does the nginx config live?

@kristjanmik
Copy link
Member

@koddsson the nginx config is not shared. I aggree with @MiniGod we should query the port internally and expose it as an endpoint

@koddsson
Copy link
Member Author

Jeee!

@koddsson
Copy link
Member Author

Could you do that real quick? 🙏

@kristjanmik
Copy link
Member

Sure

@koddsson
Copy link
Member Author

You da 💣 !

kristjanmik added a commit that referenced this pull request Feb 15, 2016
koddsson added a commit that referenced this pull request Feb 15, 2016
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