Skip to content

Rocketmq metadata and test refactor#15834

Merged
trask merged 6 commits intoopen-telemetry:mainfrom
jaydeluca:metadata-rocketmq
Jan 13, 2026
Merged

Rocketmq metadata and test refactor#15834
trask merged 6 commits intoopen-telemetry:mainfrom
jaydeluca:metadata-rocketmq

Conversation

@jaydeluca
Copy link
Copy Markdown
Member

@jaydeluca jaydeluca commented Jan 11, 2026

Part of #14096 and #14128

notes:

  • There was an unnecessary nesting (rocketmq:rocketmq-client:rocketmq-client-{version}) so I removed the redundant rocketmq-client level.
  • I needed to add a short sleep to the tests to get them to pass locally
  • I am going to continue documenting the configuration options as they were before the recent declarative config refactor, as I'm still working out how to handle the new declarative config paradigm in the context of metadata, and will need to go through and redo all the modules soon to account for that
  • I know our nightly job keeps running out of disk space and adding additional modules is not going to make that situation any better, but I have something in the works that I'm still testing to help with that

@jaydeluca jaydeluca requested a review from a team as a code owner January 11, 2026 12:21
@jaydeluca jaydeluca mentioned this pull request Jan 11, 2026
Closed
breedx-splk
breedx-splk reviewed Jan 13, 2026
View reviewed changes
Comment thread ...c/main/java/io/opentelemetry/instrumentation/rocketmqclient/v5_0/RocketMqProxyContainer.java
container.start();
try {
// Wait a bit for the broker to fully start
Thread.sleep(5000);
Copy link
Copy Markdown
Contributor

@breedx-splk breedx-splk Jan 13, 2026

Choose a reason for hiding this comment

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

That's unfortunate. It would be nicer if there were a way to poll for readiness instead of blindly sleeping. Doesn't testcontainers have a mechanism for this?

Copy link
Copy Markdown
Member Author

@jaydeluca jaydeluca Jan 13, 2026

Choose a reason for hiding this comment

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

I did try and fix it using a Wait condition, trying to find some log that could be used, but there are several processes spun up, and i didn't have much success finding a good log. With that being said, I don't think we ever see this issue in CI, and i think the wackyness is only because im on ARM. I can back this piece out since it's not critical

breedx-splk
breedx-splk approved these changes Jan 13, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@breedx-splk breedx-splk left a comment

Choose a reason for hiding this comment

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

The metadata and move/refactor seem fine to me. I do hesitate to approve with a sleep in the tests...because I think that's a slippery slope that ends up slowing down an already slow test suite....but we can always circle back to that.

@trask trask merged commit b2c97b1 into open-telemetry:main Jan 13, 2026
85 checks passed
@jaydeluca jaydeluca mentioned this pull request Mar 26, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@breedx-splk breedx-splk breedx-splk approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jaydeluca @breedx-splk @trask