Skip to content

Conversation

@JinwooHwang
Copy link
Contributor

@JinwooHwang JinwooHwang commented Jun 9, 2025

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

  • Has your PR been rebased against the latest commit within the target branch (typically develop)?

  • Is your initial contribution a single, squashed commit?

  • Does gradlew build run cleanly?

  • Have you written or updated unit tests to verify your changes?

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

@semioticrobotic
Copy link
Contributor

@niallkp: As per mailing list discussion, any chance you're able to review this?

@scmbuildguy
Copy link

@JinwooHwang-SAS Looks like there are build failures, do you think they are related to your changes?

@JinwooHwang
Copy link
Contributor Author

@scmbuildguy , I see -Werror in the build option which causes errors on warnings such as this deprecation message:
/home/runner/work/geode/geode/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/lifecycle/StopServerCommand.java:17: warning: [deprecation] StringUtils in io.micrometer.core.instrument.util has been deprecated

import static io.micrometer.core.instrument.util.StringUtils.isNotBlank;
^

Task :geode-gfsh:compileJava
error: warnings found and -Werror specified

@JinwooHwang
Copy link
Contributor Author

@niallkp and @scmbuildguy, I would appreciate it if you could kindly provide your response at your earliest convenience. Thank you.

@JinwooHwang JinwooHwang requested a review from raboof August 27, 2025 09:59
@raboof
Copy link
Member

raboof commented Aug 27, 2025

If you click the failing develop / build, you will see logs of the failure, including:

> Task :geode-gfsh:compileJava
/home/runner/work/geode/geode/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/lifecycle/StopServerCommand.java:17: warning: [deprecation] StringUtils in io.micrometer.core.instrument.util has been deprecated
import static io.micrometer.core.instrument.util.StringUtils.isNotBlank;
                                                ^
error: warnings found and -Werror specified
1 error
1 warning

@JinwooHwang
Copy link
Contributor Author

Thank you for pointing that out and for sharing the detailed logs, @raboof. I really appreciate your guidance. I’ll review the develop / build failure carefully and see what steps we can take to address it. Your insights are very helpful in moving this forward.

Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

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

OK if CI agrees

@JinwooHwang
Copy link
Contributor Author

Thank you @raboof

@raboof
Copy link
Member

raboof commented Aug 29, 2025

The IntegrationTest failure seems (at least partly) related to this PR:

    elements not found:
      ["lib/HdrHistogram-2.1.12.jar", "lib/commons-io-2.11.0.jar"]
    and elements not expected:
      ["lib/HdrHistogram-2.2.2.jar",
        "lib/commons-io-2.15.1.jar",
        "lib/micrometer-commons-1.14.0.jar",
        "lib/micrometer-observation-1.14.0.jar"]
        at org.apache.geode.AssemblyContentsIntegrationTest.verifyAssemblyContents(AssemblyContentsIntegrationTest.java:67)

@JinwooHwang
Copy link
Contributor Author

Thank you for bringing the IntegrationTest failure to my attention, @raboof. Let me investigate the issue in detail in order to determine the appropriate fix. I truly appreciate your careful review.

@semioticrobotic
Copy link
Contributor

@JinwooHwang: Did merging this PR ultimately become unnecessary? Should this work be addressed, merged, or closed?

@JinwooHwang
Copy link
Contributor Author

I am so sorry @semioticrobotic. I did not have time for this project due to the migration projects. Let me work on this after the other PR get merged. Thank you.

JinwooHwang and others added 9 commits October 22, 2025 20:21
This commit updates the expected dependency classpaths in integration tests
to reflect the addition of Micrometer dependencies to the Apache Geode build.

Changes include:
- Add micrometer-observation-1.14.0.jar to bundled jars
- Add micrometer-commons-1.14.0.jar to bundled jars
- Upgrade HdrHistogram from 2.1.12 to 2.2.2 (transitive dependency)

Updated files:
- geode-assembly/src/integrationTest/resources/expected_jars.txt
  * Updated bundled jars list for BundledJarsJUnitTest
- geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt
  * Updated gfsh-dependencies.jar manifest classpath for GfshDependencyJarIntegrationTest
- geode-server-all/src/integrationTest/resources/dependency_classpath.txt
  * Updated geode-server-all.jar manifest classpath for GeodeServerAllJarIntegrationTest

These changes ensure all assembly integration tests pass with the new
Micrometer metrics dependencies introduced in the micrometer branch.

Tests verified:
- AssemblyContentsIntegrationTest.verifyAssemblyContents ✓
- BundledJarsJUnitTest.verifyBundledJarsHaveNotChanged ✓
- GfshDependencyJarIntegrationTest.verifyManifestClassPath ✓
- GeodeServerAllJarIntegrationTest.verifyManifestClassPath ✓
Add micrometer-observation and micrometer-commons to the expected
assembly contents and update HdrHistogram from 2.1.12 to 2.2.2.

This fixes the AssemblyContentsIntegrationTest failure on CI server.

Changes:
- lib/HdrHistogram-2.1.12.jar → lib/HdrHistogram-2.2.2.jar
- Added lib/micrometer-commons-1.14.0.jar
- Added lib/micrometer-observation-1.14.0.jar (micrometer-core was already present)

Test verified: AssemblyContentsIntegrationTest.verifyAssemblyContents ✓
Synchronize dependency versions across test resource files:
- HdrHistogram: 2.2.2 → 2.1.12 (in assembly_content.txt)
- swagger-annotations: 2.2.1 → 2.2.22
- commons-io: 2.15.1 → 2.18.0
- joda-time: 2.10.14 → 2.12.7

These updates reflect the actual dependency versions in the current build
and ensure all integration tests pass.

Tests verified:
- AssemblyContentsIntegrationTest.verifyAssemblyContents ✓
- GeodeServerAllJarIntegrationTest.verifyManifestClassPath ✓
Revert HdrHistogram back to 2.2.2 to match the actual assembly build.
The CI server builds with HdrHistogram 2.2.2, not 2.1.12.

This fixes the AssemblyContentsIntegrationTest failure on CI.
Update swagger-annotations from 2.2.1 to 2.2.22 to match the actual
dependency version in the build.

This fixes the GfshDependencyJarIntegrationTest failure on CI.
- Updated HdrHistogram from 2.1.12 to 2.2.2 (following upstream changes)
- Kept micrometer 1.14.0 dependencies (observation and commons)
- Resolved assembly content and dependency classpath conflicts
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.

4 participants