Skip to content
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

otelmongo: Use a mock deployment for testing against a MongoDB server #5749

Merged
merged 32 commits into from
Sep 20, 2024

Conversation

prestonvasquez
Copy link
Contributor

@prestonvasquez prestonvasquez commented Jun 10, 2024

Resolves #39

Use mtest to mock a mongodb deployment.

@prestonvasquez prestonvasquez changed the title otelmongo#39 Convert to unit tests otelmongo#39 Use a mock deployment for testing against a MongoDB server Jun 10, 2024
@prestonvasquez prestonvasquez changed the title otelmongo#39 Use a mock deployment for testing against a MongoDB server [WIP] otelmongo#39 Use a mock deployment for testing against a MongoDB server Jun 10, 2024
CHANGELOG.md Outdated Show resolved Hide resolved
@dmathieu dmathieu added the Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG label Jun 10, 2024
Copy link

codecov bot commented Jun 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.5%. Comparing base (2a72871) to head (dd36791).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #5749     +/-   ##
=======================================
+ Coverage   65.8%   66.5%   +0.7%     
=======================================
  Files        205     205             
  Lines      13097   13093      -4     
=======================================
+ Hits        8618    8708     +90     
+ Misses      4215    4118     -97     
- Partials     264     267      +3     

see 5 files with indirect coverage changes

@prestonvasquez prestonvasquez changed the title [WIP] otelmongo#39 Use a mock deployment for testing against a MongoDB server otelmongo#39 Use a mock deployment for testing against a MongoDB server Jun 11, 2024
@prestonvasquez prestonvasquez marked this pull request as ready for review June 11, 2024 02:41
@prestonvasquez prestonvasquez requested a review from a team June 11, 2024 02:41
Copy link
Member

@dmathieu dmathieu left a comment

Choose a reason for hiding this comment

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

This is looking good to me, with the remaining question of the license.
I'll bring this topic to the SIG meeting later today.

prestonvasquez and others added 2 commits June 13, 2024 10:12
…st/mongo_test.go

Co-authored-by: Damien Mathieu <42@dmathieu.com>
…st/mongo_test.go

Co-authored-by: Damien Mathieu <42@dmathieu.com>
@dmathieu
Copy link
Member

We discussed the licensing issue during the SIG meeting on thursday, and I brought the question to the #otel-maintainers slack channel.

What it seems other languages are doing is include both licenses, the original one as well as the OpenTelemetry one.
See java-instrumentation, or the collector.

Would having something similar work for you?

@prestonvasquez
Copy link
Contributor Author

prestonvasquez commented Jun 26, 2024

We discussed the licensing issue during the SIG meeting on thursday, and I brought the question to the #otel-maintainers slack channel.

What it seems other languages are doing is include both licenses, the original one as well as the OpenTelemetry one. See java-instrumentation, or the collector.

Would having something similar work for you?

@blink1073 @matthewdale Do you have thoughts on this? This PR proposes copying code from opmsg_deployment.go to mock a server for testing. The ask is that we include both our license as well as the OpenTelemetry license in that copy.

Alternatively, we could use mtest. The problems with this solution are described in this PR here. However, it would resolve both this issue and a requirement to test the mock deployment.

.github/codecov.yaml Outdated Show resolved Hide resolved
@blink1073
Copy link

The ask is that we include both our license as well as the OpenTelemetry license in that copy.

If possible, I'd recommend keeping the original LICENSE and adding a THIRD-PARTY-NOTICES file.

@jtazin
Copy link

jtazin commented Aug 26, 2024

Hi @dmathieu @pellared @MrAlias, @prestonvasquez has reworked the code to import mtest in order to satisfy both OpenTelemetry's and MongoDB's licensing requirements. Could you please take a look and let us know if it looks good to you?

Copy link
Member

@dmathieu dmathieu left a comment

Choose a reason for hiding this comment

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

This looks nice.

.github/workflows/ci.yml Show resolved Hide resolved
@jtazin
Copy link

jtazin commented Sep 16, 2024

@pellared could you please take a look at Preston's latest changes? let us know if anything else is needed!

@pellared pellared changed the title otelmongo#39 Use a mock deployment for testing against a MongoDB server otelmongo: Use a mock deployment for testing against a MongoDB server Sep 17, 2024
@pellared
Copy link
Member

@prestonvasquez, can you sync the branch with main?

@open-telemetry/go-approvers, last call for review. I plan to merge this PR tomorrow unless nobody adds comments.

@prestonvasquez prestonvasquez requested a review from a team as a code owner September 19, 2024 16:17
@pellared pellared merged commit 4122c4b into open-telemetry:main Sep 20, 2024
25 checks passed
@MrAlias MrAlias added this to the v1.31.0 milestone Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add testing to the MongoDB plugin
6 participants