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

Add metrics for events statements by digest #19

Merged
merged 1 commit into from Aug 28, 2015

Conversation

Projects
None yet
3 participants
@SuperQ
Copy link
Member

commented Aug 24, 2015

Read metrics from performance_schema.events_statements_summary_by_digest

  • Limit to the top digests by count (default to 250)
  • Disabled by default becuase it can produce a large number of metrics

@SuperQ SuperQ changed the title Add metrics for events statements by digest [WIP] Add metrics for events statements by digest Aug 24, 2015

@@ -107,6 +111,41 @@ var (
"The total time of index I/O wait events for each index and operation.",
[]string{"schema", "name", "index", "operation"}, nil,
)
performanceSchemaEventsStatementsDesc = prometheus.NewDesc(
prometheus.BuildFQName(namespace, performanceSchema, "events_statements_total"),
"The total count of events statements by digest",

This comment has been minimized.

Copy link
@juliusv

juliusv Aug 24, 2015

Member

Some of the metric descriptions end with a period, others don't. It'd be great to make that consistent. (I prefer with period FWIW)

@SuperQ SuperQ force-pushed the bjk/events_statements branch from b9a76dc to 1040c65 Aug 24, 2015


if *perfEventsStatements {
// Timers here are returned in picoseconds.
perfSchemaEventsStatementsRows, err := db.Query("SELECT ifnull(SCHEMA_NAME, 'NONE') as SCHEMA_NAME, DIGEST, COUNT_STAR, SUM_TIMER_WAIT, SUM_ERRORS, SUM_WARNINGS, SUM_ROWS_AFFECTED, SUM_ROWS_SENT, SUM_ROWS_EXAMINED FROM performance_schema.events_statements_summary_by_digest WHERE SCHEMA_NAME NOT IN ('mysql', 'performance_schema')")

This comment has been minimized.

Copy link
@juliusv

juliusv Aug 24, 2015

Member

How about formatting statements something like this:

perfSchemaEventsStatementsRows, err := db.Query(`
  SELECT
    ifnull(SCHEMA_NAME, 'NONE') as SCHEMA_NAME,
    DIGEST,
    COUNT_STAR,
    SUM_TIMER_WAIT,
    SUM_ERRORS,
    SUM_WARNINGS,
    SUM_ROWS_AFFECTED,
    SUM_ROWS_SENT,
    SUM_ROWS_EXAMINED
  FROM
    performance_schema.events_statements_summary_by_digest
  WHERE
    SCHEMA_NAME NOT IN ('mysql', 'performance_schema')
`)

This comment has been minimized.

Copy link
@SuperQ

SuperQ Aug 24, 2015

Author Member

Yea, that's possible.

@SuperQ SuperQ force-pushed the bjk/events_statements branch 7 times, most recently from 564298d to b4f9980 Aug 24, 2015

@SuperQ

This comment has been minimized.

Copy link
Member Author

commented Aug 25, 2015

@juliusv Ok, I think I got this to a sane number of metrics, at worst it adds 2250 metrics/instance by default.

@SuperQ SuperQ changed the title [WIP] Add metrics for events statements by digest Add metrics for events statements by digest Aug 25, 2015

@SuperQ SuperQ added this to the 1.0 milestone Aug 25, 2015

if *perfEventsStatements {
// Timers here are returned in picoseconds.
perfSchemaEventsStatementsRows, err := db.Query(`
SELECT

This comment has been minimized.

Copy link
@juliusv

juliusv Aug 25, 2015

Member

I'd also indent this three tabs so it sits deeper than the db.Query surrounding it.

This comment has been minimized.

Copy link
@SuperQ

SuperQ Aug 25, 2015

Author Member

I'd like to fix that in a separate PR, I plan to cleanup all those queries into the const block.

This comment has been minimized.

Copy link
@SuperQ

SuperQ Aug 25, 2015

Author Member

Oh, wait, I can do it in this one, nevermind.

@@ -45,6 +45,14 @@ var (
"collect.perf_schema.indexiowaitstime", true,
"Collect time metrics from performance_schema.table_io_waits_summary_by_index_usage",
)
perfEventsStatements = flag.Bool(
"collect.perf_schema.eventsstatements", true,

This comment has been minimized.

Copy link
@juliusv

juliusv Aug 25, 2015

Member

Since this can be pretty heavy, maybe this should be off by default?

This comment has been minimized.

Copy link
@SuperQ

SuperQ Aug 25, 2015

Author Member

Yea, I have been waffling on this. I'm going to say yes, off by default for first release.

@SuperQ SuperQ force-pushed the bjk/events_statements branch from b4f9980 to 3cd9ab0 Aug 25, 2015

@SuperQ

This comment has been minimized.

Copy link
Member Author

commented Aug 25, 2015

@juliusv Ok, I've updated the default to false.

@SuperQ

This comment has been minimized.

Copy link
Member Author

commented Aug 26, 2015

@beorn7 More metrics!

@SuperQ SuperQ force-pushed the bjk/events_statements branch from 3cd9ab0 to 4947c02 Aug 28, 2015

@SuperQ

This comment has been minimized.

Copy link
Member Author

commented Aug 28, 2015

@fabxc More to review :) :)

@@ -46,6 +46,14 @@ var (
"collect.perf_schema.indexiowaitstime", false,
"Collect time metrics from performance_schema.table_io_waits_summary_by_index_usage",
)
perfEventsStatements = flag.Bool(
"collect.perf_schema.eventsstatements", false,

This comment has been minimized.

Copy link
@fabxc

fabxc Aug 28, 2015

Member

Is this a one-to-one mapping to an existing MySQL name? Otherwise eventsstatements sounds like a double plural.

This comment has been minimized.

Copy link
@SuperQ

SuperQ Aug 28, 2015

Author Member

Yes, the table name (see the next line) is performance_schema.events_statements_history. :-/

@@ -696,6 +767,77 @@ func (e *Exporter) scrape(ch chan<- prometheus.Metric) {
}
}

if *perfEventsStatements {
// Timers here are returned in picoseconds.
perfSchemaEventsStatementsRows, err := db.Query(perfSchemaEventsStatementsQuery + strconv.Itoa(*perfEventsStatementsLimit))

This comment has been minimized.

Copy link
@fabxc

fabxc Aug 28, 2015

Member

I suggest putting this into a fmt.Sprintf. Otherwise changing the statement text above will break down here.

schemaName string
digest string
count int64
time int64

This comment has been minimized.

Copy link
@fabxc

fabxc Aug 28, 2015

Member

This overrides the time package namespace. Works but not recommendable.

schemaName, digest,
)
ch <- prometheus.MustNewConstMetric(
performanceSchemaEventsStatementsTimeDesc, prometheus.CounterValue, float64(time)/1000000000,

This comment has been minimized.

Copy link
@fabxc

fabxc Aug 28, 2015

Member

1e9 saves people from counting zeros here.

@fabxc

This comment has been minimized.

Copy link
Member

commented Aug 28, 2015

Looks good otherwise.

@SuperQ SuperQ force-pushed the bjk/events_statements branch 2 times, most recently from d0c674f to 681ec4f Aug 28, 2015

@SuperQ

This comment has been minimized.

Copy link
Member Author

commented Aug 28, 2015

@fabxc Ok, fixed the two issues you pointed out.

@SuperQ SuperQ force-pushed the bjk/events_statements branch from 681ec4f to aaf24aa Aug 28, 2015

@@ -696,6 +766,78 @@ func (e *Exporter) scrape(ch chan<- prometheus.Metric) {
}
}

if *perfEventsStatements {
perfQuery := fmt.Sprintf("%s LIMIT %s", perfSchemaEventsStatementsQuery, strconv.Itoa(*perfEventsStatementsLimit))

This comment has been minimized.

Copy link
@fabxc

fabxc Aug 28, 2015

Member

Ah, I thought about adding a %d into the perfSchemaEventsStatementsQuery but this works just as well.
You can replace the second %s with %d and remove the call to strconv.Itoa.

This comment has been minimized.

Copy link
@SuperQ

SuperQ Aug 28, 2015

Author Member

Oh, derp, yea, good idea.

@fabxc

This comment has been minimized.

Copy link
Member

commented Aug 28, 2015

👍 after last comment.

Add metrics for events statements by digest
Read metrics from performance_schema.events_statements_summary_by_digest
* Limit to the top digests by count (default to 250)
* Disabled by default becuase it can produce a large number of metrics

@SuperQ SuperQ force-pushed the bjk/events_statements branch from aaf24aa to 9673ed7 Aug 28, 2015

SuperQ added a commit that referenced this pull request Aug 28, 2015

Merge pull request #19 from prometheus/bjk/events_statements
Add metrics for events statements by digest

@SuperQ SuperQ merged commit 71c1521 into master Aug 28, 2015

@SuperQ SuperQ deleted the bjk/events_statements branch Aug 28, 2015

SuperQ added a commit that referenced this pull request Aug 31, 2015

Cut 0.9.0 release
Includes many PRs:
* #10
* #12
* #14
* #15
* #17
* #19
* #20
* #21
* #22

@SuperQ SuperQ referenced this pull request Aug 31, 2015

Merged

Cut 0.3.0 release #24

SuperQ added a commit that referenced this pull request Aug 31, 2015

Cut 0.9.0 release
Includes many PRs:
* #10
* #12
* #14
* #15
* #17
* #19
* #20
* #21
* #22

SuperQ added a commit that referenced this pull request Aug 31, 2015

Cut 0.9.0 release
Includes many PRs:
* #10
* #12
* #14
* #15
* #17
* #19
* #20
* #21
* #22

SuperQ added a commit that referenced this pull request Aug 31, 2015

Cut 0.9.0 release
Includes many PRs:
* #10
* #12
* #14
* #15
* #17
* #19
* #20
* #21
* #22
* #25

SuperQ added a commit that referenced this pull request Aug 31, 2015

Cut 0.9.0 release
Includes many PRs:
* #10
* #12
* #14
* #15
* #17
* #19
* #20
* #21
* #22
* #25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.