-
Notifications
You must be signed in to change notification settings - Fork 453
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
Integration tests for Logs #1759
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1759 +/- ##
=====================================
Coverage 73.6% 73.6%
=====================================
Files 124 124
Lines 19517 19517
=====================================
+ Hits 14377 14378 +1
+ Misses 5140 5139 -1 ☔ View full report in Codecov by Sentry. |
tokio = { version = "1.0", features = ["full"] } | ||
serde_json = "1" | ||
testcontainers = "0.15.0" | ||
# env_logger = "0.11.3" // uncomment if needed for local debugging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't be usable further for local debugging. As we can only have one subscriber with log
.
for path in ["common.v1.KeyValue.value", "logs.v1.LogRecord.body"] { | ||
builder = builder | ||
.field_attribute(path, "#[cfg_attr(feature =\"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_to_value\", deserialize_with = \"crate::proto::serializers::deserialize_from_value\"))]"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LogRecord::body is AnyValue
, and we need a custom serializer/deserializer for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep that's a good follow up but not necessary blocking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this has been added to this PR, as was a pre-requisite to serialize/deserialize LogData.
Co-authored-by: Zhongyang Wu <[email protected]>
Changes
This is ready for review. It tests the basic use-case of OTLP Log export using default (grpc) exporter, and compare the generated log file, with one stored in the repo.
The changes are inspired (actually shameless copy :) ) from @TommyCpp 's implementation for span's integration tests.
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes