fix: logging for flaky MySQLSchema E2E test#3198
fix: logging for flaky MySQLSchema E2E test#3198csviri merged 4 commits intooperator-framework:nextfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
csviri
changed the title
There was a problem hiding this comment.
Pull request overview
Updates the MySQL schema sample operator’s E2E test and related runtime/test logging to improve stability and diagnostics.
Changes:
- Switch E2E assertions from Hamcrest to AssertJ.
- Adjust Log4j2 configuration (root level and targeted logger).
- Add a debug log line when deleting schemas and add AssertJ as a test dependency.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java | Replaces Hamcrest assertions with AssertJ assertions in the E2E test. |
| sample-operators/mysql-schema/src/main/resources/log4j2.xml | Changes logging configuration (adds a named configuration, introduces a package logger, reduces root log level). |
| sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SchemaDependentResource.java | Adds a debug log statement during schema deletion. |
| sample-operators/mysql-schema/pom.xml | Adds AssertJ as an explicit test dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Configuration name="TestConfig" status="WARN"> | ||
| <Appenders> | ||
| <Console name="Console" target="SYSTEM_OUT"> | ||
| <PatternLayout pattern="%d %threadId %-30c{1.} [%-5level] %msg p:[%X{resource.name}:%X{resource.namespace}:%X{resource.resourceVersion}] e:[%X{eventsource.name}:%X{eventsource.event.action}:%X{eventsource.event.resource.name}:%X{eventsource.event.resource.namespace}:%X{eventsource.event.resource.resourceVersion}] %n%throwable"/> | ||
| </Console> | ||
| </Appenders> | ||
| <Loggers> | ||
| <Root level="debug"> | ||
| <Logger level="debug" name="io.javaoperatorsdk.operator" additivity="false"> | ||
| <AppenderRef ref="Console"/> | ||
| </Logger> | ||
| <Root level="info"> | ||
| <AppenderRef ref="Console"/> |
There was a problem hiding this comment.
sample-operators/mysql-schema/src/main/resources/log4j2.xml is a runtime (main) resource, but the configuration is now named TestConfig and changes the global logging behavior (root debug -> info). This diverges from the other sample operators’ main Log4j2 configs (e.g. sample-operators/leader-election/src/main/resources/log4j2.xml:19-27 uses an unnamed <Configuration> with root debug) and may make debugging the sample harder. If this change is intended only for stabilizing tests, consider moving it to src/test/resources/log4j2.xml (so it only affects tests), or keep main logging consistent and add targeted logger overrides instead.
There was a problem hiding this comment.
ahh, yes forgot to push the change for this, thank you, now should look better, actually might update the leader election sample, since root logger should not be on debug.
...hema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SchemaDependentResource.java
Show resolved
Hide resolved
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
| <Configuration name="TestConfig" status="WARN"> | ||
| <Appenders> | ||
| <Console name="Console" target="SYSTEM_OUT"> | ||
| <PatternLayout pattern="%d %threadId %-30c{1.} [%-5level] %msg p:[%X{resource.name}:%X{resource.namespace}:%X{resource.resourceVersion}] e:[%X{eventsource.name}:%X{eventsource.event.action}:%X{eventsource.event.resource.name}:%X{eventsource.event.resource.namespace}:%X{eventsource.event.resource.resourceVersion}] %n%throwable"/> | ||
| </Console> | ||
| </Appenders> | ||
| <Loggers> | ||
| <Root level="debug"> | ||
| <Logger level="debug" name="io.javaoperatorsdk.operator" additivity="false"> | ||
| <AppenderRef ref="Console"/> | ||
| </Logger> | ||
| <Root level="info"> | ||
| <AppenderRef ref="Console"/> |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @Override | ||
| public void delete(MySQLSchema primary, Context<MySQLSchema> context) { | ||
| log.debug("Deleting schema"); |
There was a problem hiding this comment.
The new debug log line is quite generic and may not help diagnose which resource is being deleted during flaky runs. Consider including identifying details (e.g., schema name/namespace and possibly username/secret when available) so the log can be correlated to a specific reconciliation/deletion event.
There was a problem hiding this comment.
Note that those are already included from MDC
Signed-off-by: Attila Mészáros a_meszaros@apple.com