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

innodb status metrics for mysql reciever #54

Closed
wants to merge 13 commits into from

Conversation

naman47vyas
Copy link

Description:
ENG: 2320
Added support for new Innodb metrics. Used a new parser as a package for parsing these innodb status metrics. The parser package also has a exporter that can export the entries of metrics for the metadata.yaml, so that we don't need to manually make those entries.

Testing::
Added scrapper tests for successful scrapes.

@naman47vyas
Copy link
Author

There is a dependency of this PR, innoParser, which is used to parse the metrics and generate the metadata entries. Once a PR for that parser is merged we'll need to change go.mod files to point to the innoParser's main branch.

receiver/mysqlreceiver/client.go Outdated Show resolved Hide resolved
total_errs := 0
for key, val := range errs {
if errs[key][0] != nil {
fmt.Println(val)

Choose a reason for hiding this comment

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

No fmt.Println. Either use a proper logging library or let caller handle displaying of error.

Copy link
Author

Choose a reason for hiding this comment

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

Sure I'll change the print statements, to proper logging statements.

Copy link
Author

Choose a reason for hiding this comment

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

Now we are returning all the errors, and let scraper handle error.

return
}

fmt.Println(innodbStatusStats)

Choose a reason for hiding this comment

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

Again, no fmt.Println

@naman47vyas
Copy link
Author

For a few last commits. We have came up with a way to handle errors. The getInnodbStatusStats() method has two types of errors.
1.) Errors that should cause panic. These are the ones where there is an error with the sql client or while creating a parser itself.
2.) Errors that should not cause a panic. These are the one while parsing a particular metric. For instance if a metric named "Innodb_pending_normal_aio_reads" is failing to get recorded then there would be an entry in the "errs" map with a key as same as the metric name. Now we do accumulate and log this metrics but these will not cause panic.

@naman47vyas
Copy link
Author

Also the Parser is now merged in the Innoparser repo, so need to change the go.mod to point at the main branch instead of the "adding-parser-WIP" branch.

@naman47vyas naman47vyas requested a review from tejaskokje-mw May 20, 2024 09:08
Copy link

github-actions bot commented Jun 4, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jun 4, 2024
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants