Skip to content

feat: replace OtlpPipeline with exporter builders #2221

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

Merged
merged 13 commits into from
Oct 24, 2024
Merged

feat: replace OtlpPipeline with exporter builders #2221

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b836ece
feat: replace OtlpPipeline with exporter builders
pitoniak32 Oct 19, 2024
98d3e87
chore: remove redundant cfg feature attributes
pitoniak32 Oct 20, 2024
dbdc440
Merge branch 'main' into feat/normalize-otlp-crate
lalitb Oct 21, 2024
fa1cdb8
feat: make endpoint resolution responsibility more explicit
pitoniak32 Oct 21, 2024
dadf93b
chore: fix example to use original exporter type
pitoniak32 Oct 21, 2024
f8dd808
chore: add breaking entry to changelog
pitoniak32 Oct 21, 2024
43d5857
doc: remove mention of new_pipeline and new_exporter
pitoniak32 Oct 21, 2024
68c0fb1
Merge branch 'main' into feat/normalize-otlp-crate
cijothomas Oct 22, 2024
12ea48b
Merge branch 'main' into feat/normalize-otlp-crate
cijothomas Oct 23, 2024
563c780
Merge branch 'main' into feat/normalize-otlp-crate
cijothomas Oct 23, 2024
6e27d29
Merge branch 'main' into feat/normalize-otlp-crate
cijothomas Oct 23, 2024
8070e45
Merge branch 'main' into feat/normalize-otlp-crate
lalitb Oct 23, 2024
d6ab3de
Merge branch 'main' into feat/normalize-otlp-crate
cijothomas Oct 24, 2024
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
40 changes: 23 additions & 17 deletions examples/self-diagnostics/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use opentelemetry::global::{self, set_error_handler, Error as OtelError};
use opentelemetry::KeyValue;
use opentelemetry_appender_tracing::layer;
use opentelemetry_otlp::WithExportConfig;
use opentelemetry_otlp::{LogExporter, MetricsExporter, WithExportConfig};
use opentelemetry_sdk::metrics::PeriodicReader;
use tracing_subscriber::filter::{EnvFilter, LevelFilter};
use tracing_subscriber::fmt;
use tracing_subscriber::prelude::*;
Expand Down Expand Up @@ -51,15 +52,16 @@ fn custom_error_handler(err: OtelError) {
}

fn init_logger_provider() -> opentelemetry_sdk::logs::LoggerProvider {
let provider = opentelemetry_otlp::new_pipeline()
.logging()
.with_exporter(
opentelemetry_otlp::new_exporter()
.http()
.with_endpoint("http://localhost:4318/v1/logs"),
)
.install_batch(opentelemetry_sdk::runtime::Tokio)
let exporter = LogExporter::builder()
.with_http()
.with_endpoint("http://localhost:4318/v1/logs")
.build()
.unwrap();

let provider = opentelemetry_sdk::logs::LoggerProvider::builder()
.with_batch_exporter(exporter, opentelemetry_sdk::runtime::Tokio)
.build();

let cloned_provider = provider.clone();

// Add a tracing filter to filter events from crates used by opentelemetry-otlp.
Expand Down Expand Up @@ -107,16 +109,20 @@ fn init_logger_provider() -> opentelemetry_sdk::logs::LoggerProvider {
}

fn init_meter_provider() -> opentelemetry_sdk::metrics::SdkMeterProvider {
let provider = opentelemetry_otlp::new_pipeline()
.metrics(opentelemetry_sdk::runtime::Tokio)
.with_period(std::time::Duration::from_secs(1))
.with_exporter(
opentelemetry_otlp::new_exporter()
.http()
.with_endpoint("http://localhost:4318/v1/metrics"),
)
let exporter = MetricsExporter::builder()
.with_tonic()
.with_endpoint("http://localhost:4318/v1/metrics")
.build()
.unwrap();

let reader = PeriodicReader::builder(exporter, opentelemetry_sdk::runtime::Tokio)
.with_interval(std::time::Duration::from_secs(1))
.build();

let provider = opentelemetry_sdk::metrics::SdkMeterProvider::builder()
.with_reader(reader)
.build();

let cloned_provider = provider.clone();
global::set_meter_provider(cloned_provider);
provider
Expand Down
19 changes: 9 additions & 10 deletions examples/tracing-jaeger/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,26 @@ use opentelemetry::{
trace::{TraceContextExt, TraceError, Tracer},
KeyValue,
};
use opentelemetry_otlp::WithExportConfig;
use opentelemetry_sdk::trace::TracerProvider;
use opentelemetry_sdk::{runtime, trace as sdktrace, Resource};
use opentelemetry_semantic_conventions::resource::SERVICE_NAME;

use std::error::Error;

fn init_tracer_provider() -> Result<opentelemetry_sdk::trace::TracerProvider, TraceError> {
opentelemetry_otlp::new_pipeline()
.tracing()
.with_exporter(
opentelemetry_otlp::new_exporter()
.tonic()
.with_endpoint("http://localhost:4317"),
)
.with_trace_config(
let exporter = opentelemetry_otlp::SpanExporter::builder()
.with_tonic()
.build()?;

Ok(TracerProvider::builder()
.with_batch_exporter(exporter, runtime::Tokio)
.with_config(
sdktrace::Config::default().with_resource(Resource::new(vec![KeyValue::new(
SERVICE_NAME,
"tracing-jaeger",
)])),
)
.install_batch(runtime::Tokio)
.build())
}

#[tokio::main]
Expand Down
75 changes: 41 additions & 34 deletions opentelemetry-otlp/examples/basic-otlp-http/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@ use opentelemetry::{
KeyValue,
};
use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge;
use opentelemetry_otlp::Protocol;
use opentelemetry_otlp::{HttpExporterBuilder, WithExportConfig};
use opentelemetry_sdk::trace::{self as sdktrace, Config};
use opentelemetry_otlp::WithExportConfig;
use opentelemetry_otlp::{LogExporter, MetricsExporter, Protocol, SpanExporter};
use opentelemetry_sdk::{
logs::LoggerProvider,
metrics::{PeriodicReader, SdkMeterProvider},
runtime,
trace::{self as sdktrace, Config, TracerProvider},
};
use opentelemetry_sdk::{
logs::{self as sdklogs},
Resource,
Expand All @@ -18,6 +23,9 @@ use tracing::info;
use tracing_subscriber::prelude::*;
use tracing_subscriber::EnvFilter;

#[cfg(feature = "hyper")]
use opentelemetry_otlp::WithHttpConfig;

#[cfg(feature = "hyper")]
mod hyper;

Expand All @@ -28,47 +36,46 @@ static RESOURCE: Lazy<Resource> = Lazy::new(|| {
)])
});

fn http_exporter() -> HttpExporterBuilder {
let exporter = opentelemetry_otlp::new_exporter().http();
fn init_logs() -> Result<sdklogs::LoggerProvider, opentelemetry::logs::LogError> {
let exporter_builder = LogExporter::builder()
.with_http()
.with_endpoint("http://localhost:4318/v1/logs")
Copy link
Member

Choose a reason for hiding this comment

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

is it possible for endpoint, protocol to take defaults, so users does not have to specify this (unless they want nondefaults)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call out, The defaults should already be getting used as they were before, including the http appending the appropriate path. I added a couple more test cases for tonic to cover this.

I also changed the type of the endpoint field to Option<String> so its more explicit that it could be empty, and its on the Builders to handle resolving it.

.with_protocol(Protocol::HttpBinary);

#[cfg(feature = "hyper")]
let exporter = exporter.with_http_client(hyper::HyperClient::default());
exporter
}
let exporter_builder = exporter_builder.with_http_client(hyper::HyperClient::default());

fn init_logs() -> Result<sdklogs::LoggerProvider, opentelemetry::logs::LogError> {
opentelemetry_otlp::new_pipeline()
.logging()
let exporter = exporter_builder.build()?;

Ok(LoggerProvider::builder()
.with_batch_exporter(exporter, runtime::Tokio)
.with_resource(RESOURCE.clone())
.with_exporter(
http_exporter()
.with_protocol(Protocol::HttpBinary) //can be changed to `Protocol::HttpJson` to export in JSON format
.with_endpoint("http://localhost:4318/v1/logs"),
)
.install_batch(opentelemetry_sdk::runtime::Tokio)
.build())
}

fn init_tracer_provider() -> Result<sdktrace::TracerProvider, TraceError> {
opentelemetry_otlp::new_pipeline()
.tracing()
.with_exporter(
http_exporter()
.with_protocol(Protocol::HttpBinary) //can be changed to `Protocol::HttpJson` to export in JSON format
.with_endpoint("http://localhost:4318/v1/traces"),
)
.with_trace_config(Config::default().with_resource(RESOURCE.clone()))
.install_batch(opentelemetry_sdk::runtime::Tokio)
let exporter = SpanExporter::builder()
.with_http()
.with_protocol(Protocol::HttpBinary) //can be changed to `Protocol::HttpJson` to export in JSON format
.with_endpoint("http://localhost:4318/v1/traces")
.build()?;
Ok(TracerProvider::builder()
.with_batch_exporter(exporter, runtime::Tokio)
.with_config(Config::default().with_resource(RESOURCE.clone()))
.build())
}

fn init_metrics() -> Result<opentelemetry_sdk::metrics::SdkMeterProvider, MetricsError> {
opentelemetry_otlp::new_pipeline()
.metrics(opentelemetry_sdk::runtime::Tokio)
.with_exporter(
http_exporter()
.with_protocol(Protocol::HttpBinary) //can be changed to `Protocol::HttpJson` to export in JSON format
.with_endpoint("http://localhost:4318/v1/metrics"),
)
let exporter = MetricsExporter::builder()
.with_http()
.with_protocol(Protocol::HttpBinary) //can be changed to `Protocol::HttpJson` to export in JSON format
.with_endpoint("http://localhost:4318/v1/metrics")
.build()?;

Ok(SdkMeterProvider::builder()
.with_reader(PeriodicReader::builder(exporter, runtime::Tokio).build())
.with_resource(RESOURCE.clone())
.build()
.build())
}

#[tokio::main]
Expand Down
56 changes: 26 additions & 30 deletions opentelemetry-otlp/examples/basic-otlp/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ use opentelemetry::{
KeyValue,
};
use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge;
use opentelemetry_otlp::{ExportConfig, WithExportConfig};
use opentelemetry_otlp::{LogExporter, MetricsExporter, SpanExporter, WithExportConfig};
use opentelemetry_sdk::logs::LoggerProvider;
use opentelemetry_sdk::metrics::{PeriodicReader, SdkMeterProvider};
use opentelemetry_sdk::trace::Config;
use opentelemetry_sdk::{runtime, trace as sdktrace, Resource};
use std::error::Error;
Expand All @@ -24,43 +26,37 @@ static RESOURCE: Lazy<Resource> = Lazy::new(|| {
});

fn init_tracer_provider() -> Result<sdktrace::TracerProvider, TraceError> {
opentelemetry_otlp::new_pipeline()
.tracing()
.with_exporter(
opentelemetry_otlp::new_exporter()
.tonic()
.with_endpoint("http://localhost:4317"),
)
.with_trace_config(Config::default().with_resource(RESOURCE.clone()))
.install_batch(runtime::Tokio)
let exporter = SpanExporter::builder()
.with_tonic()
.with_endpoint("http://localhost:4317")
.build()?;
Ok(sdktrace::TracerProvider::builder()
.with_config(Config::default().with_resource(RESOURCE.clone()))
.with_batch_exporter(exporter, runtime::Tokio)
.build())
}

fn init_metrics() -> Result<opentelemetry_sdk::metrics::SdkMeterProvider, MetricsError> {
let export_config = ExportConfig {
endpoint: "http://localhost:4317".to_string(),
..ExportConfig::default()
};
opentelemetry_otlp::new_pipeline()
.metrics(runtime::Tokio)
.with_exporter(
opentelemetry_otlp::new_exporter()
.tonic()
.with_export_config(export_config),
)
let exporter = MetricsExporter::builder().with_tonic().build()?;

let reader = PeriodicReader::builder(exporter, runtime::Tokio).build();

Ok(SdkMeterProvider::builder()
.with_reader(reader)
.with_resource(RESOURCE.clone())
.build()
.build())
}

fn init_logs() -> Result<opentelemetry_sdk::logs::LoggerProvider, LogError> {
opentelemetry_otlp::new_pipeline()
.logging()
let exporter = LogExporter::builder()
.with_tonic()
.with_endpoint("http://localhost:4317")
Copy link
Member

Choose a reason for hiding this comment

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

one inconsistency I notice between grpc and http is the end point - one has to append /v1/logs in http, but grpc does not require that. This is something we can address in a follow up.
I am thinking something like:
with_tonic() or with_http() is enough for builder to know which endpoint, protocol to use by default.
builder.with_tonic().build() -- this should just work with defaults
builder.with_http().build() -- this should also just work with defaults.

if user wish to change defaults, then they can use methods like with_endpoint, with_protocol etc.

Lets come back to this after merging this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good, after my most recent update, the with_{tonic,http}() is enough to determine the http://localhost:431{7,8} and the tests for this are updated also.

I will however need to look at the spec to see how the /v1/* is handled for the two, and I would be happy to create a new pr to get that sorted out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good, after my most recent update, the with_{tonic,http}() is enough to determine the http://localhost:431{7,8} and the tests for this are updated also.

I will however need to look at the spec to see how the /v1/* is handled for the two, and I would be happy to create a new pr to get that sorted out.

EDIT: After looking back through the code, this should also already be the case, I made some changes to make it a little more clear

.build()?;

Ok(LoggerProvider::builder()
.with_resource(RESOURCE.clone())
.with_exporter(
opentelemetry_otlp::new_exporter()
.tonic()
.with_endpoint("http://localhost:4317"),
)
.install_batch(runtime::Tokio)
.with_batch_exporter(exporter, runtime::Tokio)
.build())
}

#[tokio::main]
Expand Down
Loading
Loading