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

Enable metric integration test for req-blocking #2445

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
"shoppingcart",
"struct",
"Tescher",
"testresults",
"tracerprovider",
"updown",
"Zhongyang",
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-otlp/tests/integration_test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ testcontainers = { version = "0.23.1", features = ["http_wait"]}
once_cell.workspace = true
anyhow = "1.0.94"
ctor = "0.2.9"
tracing-subscriber = "0.3.19"
tracing-subscriber = { workspace = true, features = ["env-filter","registry", "std", "fmt"] }
tracing = "0.1.41"

[target.'cfg(unix)'.dependencies]
Expand Down
19 changes: 13 additions & 6 deletions opentelemetry-otlp/tests/integration_test/README.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
# OTLP - Integration Tests
This directory contains integration tests for `opentelemetry-otlp`. It uses
[testcontainers](https://testcontainers.com/) to start an instance of the OTEL collector using [otel-collector-config.yaml](otel-collector-config.yaml), which then uses a file exporter per signal to write the output it receives back to the host machine.

The tests connect directly to the collector on `localhost:4317` and `localhost:4318`, push data through, and then check that what they expect
has popped back out into the files output by the collector.
This directory contains integration tests for `opentelemetry-otlp`. It uses
[testcontainers](https://testcontainers.com/) to start an instance of the OTEL
collector using [otel-collector-config.yaml](otel-collector-config.yaml), which
then uses a file exporter per signal to write the output it receives back to the
host machine.

The tests connect directly to the collector on `localhost:4317` and
`localhost:4318`, push data through, and then check that what they expect has
popped back out into the files output by the collector.

## Pre-requisites

For this to work, you need a couple of things:
* Docker, for the test container
* TCP/4317 and TCP/4318 free on your local machine. If you are running another collector, you'll need to stop it for the tests to run
* TCP/4317 and TCP/4318 free on your local machine. If you are running another
collector, you'll need to stop it for the tests to run.
22 changes: 14 additions & 8 deletions opentelemetry-otlp/tests/integration_test/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ use std::sync::{Arc, Mutex, Once, OnceLock};
use testcontainers::core::wait::HttpWaitStrategy;
use testcontainers::core::{ContainerPort, Mount};
use testcontainers::{core::WaitFor, runners::AsyncRunner, ContainerAsync, GenericImage, ImageExt};
use tracing_subscriber::FmtSubscriber;
use tracing_subscriber::layer::SubscriberExt;
use tracing_subscriber::util::SubscriberInitExt;
use tracing_subscriber::{EnvFilter, Layer};

// Static references for container management
static COLLECTOR_ARC: OnceLock<Mutex<Option<Arc<ContainerAsync<GenericImage>>>>> = OnceLock::new();
Expand All @@ -40,13 +42,17 @@ static INIT_TRACING: Once = Once::new();

fn init_tracing() {
INIT_TRACING.call_once(|| {
let subscriber = FmtSubscriber::builder()
.with_max_level(tracing::Level::DEBUG)
.finish();

tracing::subscriber::set_global_default(subscriber)
.expect("Failed to set tracing subscriber");
otel_info!(name: "init_tracing");
// Info and above for all, debug for opentelemetry
let filter_fmt =
EnvFilter::new("info").add_directive("opentelemetry=debug".parse().unwrap());
let fmt_layer = tracing_subscriber::fmt::layer()
.with_thread_names(true)
.with_filter(filter_fmt);

// Initialize the tracing subscriber with the OpenTelemetry layer and the
// Fmt layer.
tracing_subscriber::registry().with(fmt_layer).init();
otel_info!(name: "tracing initializing completed!");
});
}

Expand Down
10 changes: 5 additions & 5 deletions opentelemetry-otlp/tests/integration_test/tests/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ async fn init_metrics() -> SdkMeterProvider {
let exporter = create_exporter();

let reader = PeriodicReader::builder(exporter)
.with_interval(Duration::from_millis(100))
.with_interval(Duration::from_millis(500))
Copy link
Member Author

Choose a reason for hiding this comment

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

the sleep is around 5 seconds, so 500 msec interval is fine, and this reduces the amount of internal logs.
Also, I am thinking if we should rely on force_flush/shutdown for integration tests more, to avoid this sleep requirement. We should still have one set of tests for testing the export triggered by interval.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll still need a bit of a buffer because of the flushing on the otlp-collector side? What I think what would be the nicest would be to setup a simple, short exponential backoff mechanism in here - the function knows what it is looking for for the particular test, so it's well positioned to decide to wait a bit and look again:

pub fn fetch_latest_metrics_for_scope(scope_name: &str) -> Result<Value> {

so that we can use a tighter timing in the best case.

I've also added this rotation thing for logs, which I believe will decrease buffering on the collector side - doesn't help us with the other signals, though:

file/logs:
path: /testresults/logs.json
rotation:

.with_timeout(Duration::from_secs(1))
.build();

Expand Down Expand Up @@ -146,7 +146,7 @@ async fn setup_metrics_test() -> Result<()> {
println!("Running setup before any tests...");
*done = true; // Mark setup as done

// Initialise the metrics subsystem
// Initialize the metrics subsystem
Copy link
Contributor

Choose a reason for hiding this comment

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

US-English spelling strikes again 🤣

_ = init_metrics().await;
}

Expand Down Expand Up @@ -184,13 +184,13 @@ pub fn validate_metrics_against_results(scope_name: &str) -> Result<()> {
}

///
/// TODO - the HTTP metrics exporters do not seem to flush at the moment.
/// TODO - the HTTP metrics exporters except reqwest-blocking-client do not seem
/// to work at the moment.
/// TODO - fix this asynchronously.
///
#[cfg(test)]
#[cfg(not(feature = "hyper-client"))]
#[cfg(not(feature = "reqwest-client"))]
#[cfg(not(feature = "reqwest-blocking-client"))]
mod tests {

use super::*;
Expand Down Expand Up @@ -293,7 +293,7 @@ mod tests {
// Set up the exporter
let exporter = create_exporter();
let reader = PeriodicReader::builder(exporter)
.with_interval(Duration::from_millis(100))
.with_interval(Duration::from_secs(30))
Copy link
Member Author

Choose a reason for hiding this comment

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

putting a large interval here, to make sure the test is actually testing shutdown triggered exporting, not a timer triggered.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea

.with_timeout(Duration::from_secs(1))
.build();
let resource = Resource::builder_empty()
Expand Down
Loading