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

[mysql] Allow basic replication info to be available without performance_schema available #3102

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dballing
Copy link

Note: Please remember to review the Datadog Contribution Guidelines
if you have not yet done so.

What does this PR do?

This change removes an arbitrary requirement for mysql >=5.6 if trying to view basic replication status

Motivation

We have MySQL 5.1.x, which does not have performance_schema. I wanted to monitor basic replication data, and I saw that this data was coming from SLOW SLAVE STATUS and not the performance_schema database.

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

Additional Notes

Anything else we should know when reviewing?

@masci masci added this to the Triage milestone Jan 24, 2017
@truthbk truthbk self-assigned this Jan 31, 2017
@truthbk
Copy link
Member

truthbk commented Feb 2, 2017

Hey @dballing, thank you for taking the time to look into this. The requirement for mysql>=5.6 isn't completely arbitrary :) it was done because the check doesn't officially support EOL'd MySQLs like 5.1. There were too many loose ends supporting such a wide array of versions. Your changes, however, do seem harmless across versions so we could definitely consider merging this...

Unfortunately I haven't had time to test this out properly as we are currently in the process of transitioning to an "SDK"-based model for the checks.

@irabinovitch
Copy link
Contributor

irabinovitch commented Feb 4, 2017

Hello @dballing

Thank you for your contribution and efforts on this PR. We are currently in the process of migrating agent checks from this repository into two new repositories https://github.com/DataDog/integrations-core and https://github.com/DataDog/integrations-extras in order to support a new integration release process for integrations.

The new release process which begins with dd-agent 5.12 will allow us to test and merge integrations more quickly, as well as release them as individual packages. An unfortunate side-effect is that we have to impose a freeze on integrations PRs on the dd-agent repo.

Would it be OK if we close this PR and and as you to open a new one against the integrations-core repository?

We will be keeping a close eye on these repos and will be able to offer quick feedback on any PRs you submit. Additionally we have launched a new community office hours during which our engineering and community teams will be available to perform code review and offer feedback on new and existing integration.

To help ease integration development and migration to the new release model we have developed some tools and documentation:

We hope that this short term inconvenience here will result in happier and more productive workflow for our community.

Thank you for your patience as we have been working through this transition. If you have any questions, we are happy to discuss here in this PR or on Slack in #integrations.

@irabinovitch
Copy link
Contributor

@dballing Our next community office hours and code review is tomorrow at 10:30am PT and we'd be happy to review this PR with you there if you'd like to join us and help clarify the repo split.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants