Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

Enable metrics publishing for mt-gateway #1645

Merged
merged 9 commits into from
Feb 11, 2020
Merged

Conversation

fitzoh
Copy link
Contributor

@fitzoh fitzoh commented Jan 29, 2020

As requested here.

Note that there are a couple places where prometheus instrumentation is used that have not been addressed:

discardedSamples = promauto.NewCounterVec(

ingestedMetrics = promauto.NewCounterVec(prometheus.CounterOpts{

@fitzoh fitzoh requested a review from a team January 29, 2020 00:12

// stats
statsEnabled = flag.Bool("stats-enabled", false, "enable sending graphite messages for instrumentation")
statsPrefix = flag.String("stats-prefix", "tsdb-gw.stats.default.$hostname", "stats prefix (will add trailing dot automatically if needed)")
Copy link
Contributor

Choose a reason for hiding this comment

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

the default value here does not match the one in the config file. I think it should also be mt-gateway.stats.default.$hostname

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, e5ee174

Copy link
Contributor

@replay replay left a comment

Choose a reason for hiding this comment

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

LGTM, just one very minor issue to fix

Copy link
Contributor

@replay replay left a comment

Choose a reason for hiding this comment

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

:shipit:

@Dieterbe
Copy link
Contributor

before we merge. can you confirm that you can take a tsdb-gw dashboard, s/tsdb-gw/mt-gateway/ and it works ?

@fitzoh
Copy link
Contributor Author

fitzoh commented Jan 29, 2020

Screen Shot 2020-01-29 at 10 30 40 AM

First couple look good, http stuff isn't instrumented

@Dieterbe
Copy link
Contributor

http stuff isn't instrumented

can we ?

@fitzoh
Copy link
Contributor Author

fitzoh commented Jan 29, 2020

can we ?

Copied over in c803c82... Not sure if we want to leave them as is or switch to tag based metrics.

@fitzoh fitzoh requested a review from Dieterbe January 30, 2020 15:43
Copy link
Contributor

@Dieterbe Dieterbe left a comment

Choose a reason for hiding this comment

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

how is this copied over? the code looks quite different, making it hard to compare.. can you just make a literal copy, and maybe make any needed changes in a separate commit?

also, that screenie looks good. can you confirm these stats similarly?

@fitzoh
Copy link
Contributor Author

fitzoh commented Feb 10, 2020

how is this copied over? the code looks quite different, making it hard to compare.. can you just make a literal copy, and maybe make any needed changes in a separate commit?

Initial import in 1b16b0d, changes in subsequent commits

@fitzoh
Copy link
Contributor Author

fitzoh commented Feb 10, 2020

also, that screenie looks good. can you confirm these stats similarly?

After some tweaks to get things up and running...

Screen Shot 2020-02-10 at 11 09 01 AM

The response codes were worrying me a little bit, but I was able to confirm that there's data there through the explore view. (The query the dashboard was running was groupByNode(perSecond(mt-gateway.stats.$env.$host.api.request.$path.status.*.counter32), 8, 'sum'))

Screen Shot 2020-02-10 at 11 09 59 AM

@Dieterbe Dieterbe merged commit 60eebbe into master Feb 11, 2020
@Dieterbe Dieterbe deleted the mt-gateway-metrics branch February 11, 2020 22:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants