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

Add tracing tests using inmemoryexporter #1305

Merged
merged 3 commits into from
Oct 20, 2023

Conversation

cijothomas
Copy link
Member

Adding module level tests, which tests the entire tracing api/sdk pipeline. These are not unit tests, but not integration tests either.
I think this is a good way to test things, and improve coverage. This is pretty close to testing what an end user experiences (minus the actual exporter part, which can be covered by spinning up collectors in the future.)

This was discussed in todays SIG meeting, and opening this PR to get early feedback. If all likes this direction, then I can send follow up PRs greatly improving coverage, and extend it to metrics and logs as well. (Or create issues with "help-wanted" tag inviting more folks to contribute.)

This uses in_memory_exporter which were recently added. If we ourselves don't use it, who else will 🤣 !

@cijothomas cijothomas requested a review from a team as a code owner October 17, 2023 19:33
@cijothomas
Copy link
Member Author

Tagging @hdost @TommyCpp @lalitb with whom this was discussed in the SIG call today! Open for feedback from all!

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
opentelemetry-sdk/src/trace/mod.rs 100.0% <100.0%> (ø)

... and 3 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

Copy link
Contributor

@TommyCpp TommyCpp left a comment

Choose a reason for hiding this comment

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

Small nit for simplify the test setup.

opentelemetry-sdk/src/trace/mod.rs Show resolved Hide resolved
opentelemetry-sdk/src/trace/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@shaun-cox shaun-cox left a comment

Choose a reason for hiding this comment

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

I like the direction.

opentelemetry-sdk/src/trace/mod.rs Show resolved Hide resolved
@TommyCpp TommyCpp merged commit 2fb0c2e into open-telemetry:main Oct 20, 2023
13 checks passed
@cijothomas cijothomas deleted the cijothomas/tracing-tests1 branch October 20, 2023 17:16
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.

None yet

4 participants