Skip to content

Add slowQueryLogConfigTmpl options to use custom slow-log config #810

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

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

Conversation

yamatcha
Copy link
Contributor

@yamatcha yamatcha commented Jun 2, 2025

@shunki-fujita shunki-fujita requested a review from Copilot June 4, 2025 05:37
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for custom slow query log configuration by introducing the slowQueryLogConfigTmpl option and modifying related behavior in the controller and CRDs. Key changes include:

  • Updated documentation and CRDs to include the new slowQueryLogConfigTmpl field.
  • Modified the FluentBit config map reconciliation to use text/template for processing the slow log configuration.
  • Enhanced tests to validate the custom configuration.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
docs/crd_mysqlcluster_v1beta2.md Updated field descriptions to reflect the new slowQueryLogConfigTmpl option and adjusted behavior for the disableSlowQueryLogContainer flag.
controllers/mysqlcluster_controller_test.go Added tests to verify the custom configuration is properly reconciled in the slow-log ConfigMap.
controllers/mysqlcluster_controller.go Replaced fmt.Sprintf with text/template parsing and execution to support custom slow query log templates.
config/crd/tests/apiextensions.k8s.io_v1_customresourcedefinition_mysqlclusters.moco.cybozu.com.yaml Updated CRD to include the slowQueryLogConfigTmpl field.
config/crd/bases/moco.cybozu.com_mysqlclusters.yaml Updated CRD base definitions to add the slowQueryLogConfigTmpl field.
api/v1beta2/zz_generated.deepcopy.go, api/v1beta2/mysqlcluster_types.go Added deepcopy logic and API type support for the new slowQueryLogConfigTmpl field.

return fmt.Errorf("failed to parse config template: %w", err)
}
confVal := new(bytes.Buffer)
t.Execute(confVal, struct{ Path string }{Path: filepath.Join(constants.LogDirPath, constants.MySQLSlowLogName)})
Copy link
Preview

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

Consider checking the error returned by t.Execute. Failing to capture and handle this error could lead to unnoticed issues during template execution.

Suggested change
t.Execute(confVal, struct{ Path string }{Path: filepath.Join(constants.LogDirPath, constants.MySQLSlowLogName)})
if err := t.Execute(confVal, struct{ Path string }{Path: filepath.Join(constants.LogDirPath, constants.MySQLSlowLogName)}); err != nil {
return fmt.Errorf("failed to execute config template: %w", err)
}

Copilot uses AI. Check for mistakes.

}
if !strings.Contains(slowCM.Data[constants.FluentBitConfigName], filepath.Join(constants.LogDirPath, constants.MySQLSlowLogName)) {
return fmt.Errorf("the config map is invalid")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Also check that dummyKey/value is included.

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.

2 participants