Skip to content

Add service connection from Testcontainers OpenTelemetry Collector #35082

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

Add service connection from Testcontainers OpenTelemetry Collector #35082

wants to merge 8 commits into from

Conversation

@eddumelendez
Copy link
Contributor

@eddumelendez eddumelendez commented Apr 20, 2023

No description provided.

eddumelendez
eddumelendez commented Apr 20, 2023
View reviewed changes
...ontainers/service/connection/otlp/OtlpContainerConnectionDetailsFactoryIntegrationTests.java
* Tests for {@link OpenTelemetryConnectionDetailsFactory}.
*
* @author Eddú Meléndez
* @author Jonatan Ivanov
Copy link
Contributor Author

@eddumelendez eddumelendez Apr 20, 2023

Choose a reason for hiding this comment

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

Adding Jonatan due to the test was the one he submitted in micrometer https://github.com/micrometer-metrics/micrometer/blob/main/implementations/micrometer-registry-otlp/src/test/java/io/micrometer/registry/otlp/OTelCollectorIntegrationTest.java

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 20, 2023
@eddumelendez eddumelendez force-pushed the opentelemetry_service_connection branch from 8facf3b to 8563165 Compare April 20, 2023 02:42
jonatan-ivanov
jonatan-ivanov reviewed Apr 26, 2023
View reviewed changes
Copy link
Member

@jonatan-ivanov jonatan-ivanov left a comment

Choose a reason for hiding this comment

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

We've recently added support for OtlpSpanExporter too. Do you think it would worth using OtlpConnectionDetails there too?

See: #34508

@jonatan-ivanov jonatan-ivanov added theme: observability Issues related to observability status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 26, 2023
@eddumelendez
Copy link
Contributor Author

eddumelendez commented Apr 26, 2023

@jonatan-ivanov I think it make sense 😃 PR updated

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 26, 2023
@wilkinsona wilkinsona added the theme: containers Testcontainers, Docker Compose and Buildpack features label May 2, 2023
@wilkinsona wilkinsona added this to the 3.x milestone May 3, 2023
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided labels May 3, 2023
@philwebb philwebb removed the theme: observability Issues related to observability label Jun 7, 2023
@mhalbritter
Copy link
Contributor

mhalbritter commented Sep 13, 2023 •
edited
Loading

Hey, I was just looking at that PR, sorry for taking my time. Do you mind updating it to the latest main and fixing the conflicts? Maybe we can get this into 3.2.0-M3.

Do you think it makes sense to provide connection details sourced from Docker Compose, too?

@mhalbritter mhalbritter self-assigned this Sep 13, 2023
@mhalbritter mhalbritter added the status: waiting-for-feedback We need additional information before we can continue label Sep 13, 2023
@eddumelendez
Copy link
Contributor Author

eddumelendez commented Sep 13, 2023

Hi @mhalbritter, I'll 👍🏽

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 13, 2023
@eddumelendez
Copy link
Contributor Author

eddumelendez commented Sep 13, 2023

PR updated

@mhalbritter
Copy link
Contributor

mhalbritter commented Sep 14, 2023

Thanks!

mhalbritter pushed a commit that referenced this pull request Sep 14, 2023
@eddumelendez @mhalbritter
Add service connection from OpenTelemetry Collector
6997277 
See gh-35082
mhalbritter added a commit that referenced this pull request Sep 14, 2023
@mhalbritter
Polish "Add service connection from OpenTelemetry Collector"
c387c87 
See gh-35082
@mhalbritter
Copy link
Contributor

mhalbritter commented Sep 14, 2023

Thank you very much @eddumelendez !

@mhalbritter mhalbritter modified the milestones: 3.x, 3.2.0-M3 Sep 14, 2023
@mhalbritter mhalbritter removed the status: feedback-provided Feedback has been provided label Sep 14, 2023
mhalbritter added a commit that referenced this pull request Sep 14, 2023
@mhalbritter
Polish "Add service connection from OpenTelemetry Collector"
3dd6372 
See gh-35082
izeye added a commit to izeye/spring-boot that referenced this pull request Oct 11, 2023
@izeye
Polish spring-projectsgh-35082
8985083
@izeye izeye mentioned this pull request Oct 11, 2023
Closed
mhalbritter pushed a commit that referenced this pull request Oct 12, 2023
@izeye @mhalbritter
Polish gh-35082
4466773 
See gh-37831
mhalbritter added a commit that referenced this pull request Oct 12, 2023
@mhalbritter
Merge pull request #37831 from izeye
b79e10e 
* pr/37831:
  Polish gh-35082

Closes gh-37831
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonatan-ivanov jonatan-ivanov jonatan-ivanov left review comments

Assignees

@mhalbritter mhalbritter

Labels

theme: containers Testcontainers, Docker Compose and Buildpack features type: enhancement A general enhancement

Projects

None yet

Milestone

3.2.0-M3

Development

Successfully merging this pull request may close these issues.

6 participants

@eddumelendez @mhalbritter @jonatan-ivanov @philwebb @wilkinsona @spring-projects-issues