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

Move Logs and Metrics error constructs to SDK #2266

Merged
merged 23 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
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
6 changes: 1 addition & 5 deletions examples/self-diagnostics/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use opentelemetry::global::{self, Error as OtelError};
use opentelemetry::global;
use opentelemetry::KeyValue;
use opentelemetry_appender_tracing::layer;
use opentelemetry_otlp::{LogExporter, MetricExporter, WithExportConfig};
Expand All @@ -7,10 +7,6 @@ use tracing_subscriber::prelude::*;

use std::error::Error;

use once_cell::sync::Lazy;
use std::collections::HashSet;
use std::sync::{Arc, Mutex};

use std::sync::mpsc::channel;

fn init_logger_provider() -> opentelemetry_sdk::logs::LoggerProvider {
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-appender-tracing/benches/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@

use async_trait::async_trait;
use criterion::{criterion_group, criterion_main, Criterion};
use opentelemetry::logs::LogResult;
use opentelemetry::{InstrumentationScope, KeyValue};
use opentelemetry_appender_tracing::layer as tracing_layer;
use opentelemetry_sdk::export::logs::{LogBatch, LogExporter};
use opentelemetry_sdk::logs::LogResult;
use opentelemetry_sdk::logs::{LogProcessor, LogRecord, LoggerProvider};
use opentelemetry_sdk::Resource;
use pprof::criterion::{Output, PProfProfiler};
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-appender-tracing/src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,12 @@ const fn severity_of_level(level: &Level) -> Severity {
mod tests {
use crate::layer;
use async_trait::async_trait;
use opentelemetry::logs::{LogResult, Severity};
use opentelemetry::logs::Severity;
use opentelemetry::trace::TracerProvider as _;
use opentelemetry::trace::{TraceContextExt, TraceFlags, Tracer};
use opentelemetry::{logs::AnyValue, Key};
use opentelemetry_sdk::export::logs::{LogBatch, LogExporter};
use opentelemetry_sdk::logs::{LogRecord, LoggerProvider};
use opentelemetry_sdk::logs::{LogRecord, LogResult, LoggerProvider};
use opentelemetry_sdk::testing::logs::InMemoryLogExporter;
use opentelemetry_sdk::trace;
use opentelemetry_sdk::trace::{Sampler, TracerProvider};
Expand Down
5 changes: 2 additions & 3 deletions opentelemetry-otlp/examples/basic-otlp-http/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
use once_cell::sync::Lazy;
use opentelemetry::{
global,
metrics::MetricError,
trace::{TraceContextExt, TraceError, Tracer},
InstrumentationScope, KeyValue,
};
Expand All @@ -11,7 +10,7 @@ use opentelemetry_otlp::WithExportConfig;
use opentelemetry_otlp::{LogExporter, MetricExporter, Protocol, SpanExporter};
use opentelemetry_sdk::{
logs::LoggerProvider,
metrics::{PeriodicReader, SdkMeterProvider},
metrics::{MetricError, PeriodicReader, SdkMeterProvider},
runtime,
trace::{self as sdktrace, Config, TracerProvider},
};
Expand All @@ -31,7 +30,7 @@ static RESOURCE: Lazy<Resource> = Lazy::new(|| {
)])
});

fn init_logs() -> Result<sdklogs::LoggerProvider, opentelemetry::logs::LogError> {
fn init_logs() -> Result<sdklogs::LoggerProvider, opentelemetry_sdk::logs::LogError> {
let exporter = LogExporter::builder()
.with_http()
.with_endpoint("http://localhost:4318/v1/logs")
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-otlp/examples/basic-otlp/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use once_cell::sync::Lazy;
use opentelemetry::logs::LogError;
use opentelemetry::metrics::MetricError;
use opentelemetry::trace::{TraceContextExt, TraceError, Tracer};
use opentelemetry::KeyValue;
use opentelemetry::{global, InstrumentationScope};
use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge;
use opentelemetry_otlp::{LogExporter, MetricExporter, SpanExporter, WithExportConfig};
use opentelemetry_sdk::logs::LogError;
use opentelemetry_sdk::logs::LoggerProvider;
use opentelemetry_sdk::metrics::MetricError;
use opentelemetry_sdk::metrics::{PeriodicReader, SdkMeterProvider};
use opentelemetry_sdk::trace::Config;
use opentelemetry_sdk::{runtime, trace as sdktrace, Resource};
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-otlp/src/exporter/http/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use std::sync::Arc;

use async_trait::async_trait;
use http::{header::CONTENT_TYPE, Method};
use opentelemetry::logs::{LogError, LogResult};
use opentelemetry_sdk::export::logs::{LogBatch, LogExporter};
use opentelemetry_sdk::logs::{LogError, LogResult};

use super::OtlpHttpClient;

Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-otlp/src/exporter/http/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use std::sync::Arc;

use async_trait::async_trait;
use http::{header::CONTENT_TYPE, Method};
use opentelemetry::metrics::{MetricError, MetricResult};
use opentelemetry_sdk::metrics::data::ResourceMetrics;
use opentelemetry_sdk::metrics::{MetricError, MetricResult};

use crate::{metric::MetricsClient, Error};

Expand Down
14 changes: 8 additions & 6 deletions opentelemetry-otlp/src/exporter/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@

/// Create a log exporter with the current configuration
#[cfg(feature = "logs")]
pub fn build_log_exporter(mut self) -> opentelemetry::logs::LogResult<crate::LogExporter> {
pub fn build_log_exporter(mut self) -> opentelemetry_sdk::logs::LogResult<crate::LogExporter> {

Check warning on line 226 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/http/mod.rs#L226

Added line #L226 was not covered by tests
use crate::{
OTEL_EXPORTER_OTLP_LOGS_ENDPOINT, OTEL_EXPORTER_OTLP_LOGS_HEADERS,
OTEL_EXPORTER_OTLP_LOGS_TIMEOUT,
Expand All @@ -244,7 +244,7 @@
pub fn build_metrics_exporter(
mut self,
temporality: opentelemetry_sdk::metrics::data::Temporality,
) -> opentelemetry::metrics::MetricResult<crate::MetricExporter> {
) -> opentelemetry_sdk::metrics::MetricResult<crate::MetricExporter> {

Check warning on line 247 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/http/mod.rs#L247

Added line #L247 was not covered by tests
use crate::{
OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, OTEL_EXPORTER_OTLP_METRICS_HEADERS,
OTEL_EXPORTER_OTLP_METRICS_TIMEOUT,
Expand Down Expand Up @@ -315,7 +315,7 @@
fn build_logs_export_body(
&self,
logs: LogBatch<'_>,
) -> opentelemetry::logs::LogResult<(Vec<u8>, &'static str)> {
) -> opentelemetry_sdk::logs::LogResult<(Vec<u8>, &'static str)> {

Check warning on line 318 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/http/mod.rs#L318

Added line #L318 was not covered by tests
use opentelemetry_proto::tonic::collector::logs::v1::ExportLogsServiceRequest;
let resource_logs = group_logs_by_resource_and_scope(logs, &self.resource);
let req = ExportLogsServiceRequest { resource_logs };
Expand All @@ -324,7 +324,7 @@
#[cfg(feature = "http-json")]
Protocol::HttpJson => match serde_json::to_string_pretty(&req) {
Ok(json) => Ok((json.into(), "application/json")),
Err(e) => Err(opentelemetry::logs::LogError::from(e.to_string())),
Err(e) => Err(opentelemetry_sdk::logs::LogError::from(e.to_string())),

Check warning on line 327 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/http/mod.rs#L327

Added line #L327 was not covered by tests
},
_ => Ok((req.encode_to_vec(), "application/x-protobuf")),
}
Expand All @@ -334,7 +334,7 @@
fn build_metrics_export_body(
&self,
metrics: &mut opentelemetry_sdk::metrics::data::ResourceMetrics,
) -> opentelemetry::metrics::MetricResult<(Vec<u8>, &'static str)> {
) -> opentelemetry_sdk::metrics::MetricResult<(Vec<u8>, &'static str)> {

Check warning on line 337 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/http/mod.rs#L337

Added line #L337 was not covered by tests
use opentelemetry_proto::tonic::collector::metrics::v1::ExportMetricsServiceRequest;

let req: ExportMetricsServiceRequest = (&*metrics).into();
Expand All @@ -343,7 +343,9 @@
#[cfg(feature = "http-json")]
Protocol::HttpJson => match serde_json::to_string_pretty(&req) {
Ok(json) => Ok((json.into(), "application/json")),
Err(e) => Err(opentelemetry::metrics::MetricError::Other(e.to_string())),
Err(e) => Err(opentelemetry_sdk::metrics::MetricError::Other(
e.to_string(),
)),

Check warning on line 348 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/http/mod.rs#L346-L348

Added lines #L346 - L348 were not covered by tests
},
_ => Ok((req.encode_to_vec(), "application/x-protobuf")),
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-otlp/src/exporter/tonic/logs.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use async_trait::async_trait;
use core::fmt;
use opentelemetry::logs::{LogError, LogResult};
use opentelemetry_proto::tonic::collector::logs::v1::{
logs_service_client::LogsServiceClient, ExportLogsServiceRequest,
};
use opentelemetry_sdk::export::logs::{LogBatch, LogExporter};
use opentelemetry_sdk::logs::{LogError, LogResult};
use tonic::{codegen::CompressionEncoding, service::Interceptor, transport::Channel, Request};

use opentelemetry_proto::transform::logs::tonic::group_logs_by_resource_and_scope;
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-otlp/src/exporter/tonic/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ use core::fmt;
use std::sync::Mutex;

use async_trait::async_trait;
use opentelemetry::metrics::{MetricError, MetricResult};
use opentelemetry_proto::tonic::collector::metrics::v1::{
metrics_service_client::MetricsServiceClient, ExportMetricsServiceRequest,
};
use opentelemetry_sdk::metrics::data::ResourceMetrics;
use opentelemetry_sdk::metrics::{MetricError, MetricResult};
use tonic::{codegen::CompressionEncoding, service::Interceptor, transport::Channel, Request};

use super::BoxInterceptor;
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-otlp/src/exporter/tonic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@
#[cfg(feature = "logs")]
pub(crate) fn build_log_exporter(
self,
) -> Result<crate::logs::LogExporter, opentelemetry::logs::LogError> {
) -> Result<crate::logs::LogExporter, opentelemetry_sdk::logs::LogError> {

Check warning on line 257 in opentelemetry-otlp/src/exporter/tonic/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/tonic/mod.rs#L257

Added line #L257 was not covered by tests
use crate::exporter::tonic::logs::TonicLogsClient;

let (channel, interceptor, compression) = self.build_channel(
Expand All @@ -274,7 +274,7 @@
pub(crate) fn build_metrics_exporter(
self,
temporality: opentelemetry_sdk::metrics::data::Temporality,
) -> opentelemetry::metrics::MetricResult<crate::MetricExporter> {
) -> opentelemetry_sdk::metrics::MetricResult<crate::MetricExporter> {

Check warning on line 277 in opentelemetry-otlp/src/exporter/tonic/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/tonic/mod.rs#L277

Added line #L277 was not covered by tests
use crate::MetricExporter;
use metrics::TonicMetricsClient;

Expand Down
6 changes: 6 additions & 0 deletions opentelemetry-otlp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,12 @@
}
}

impl opentelemetry::trace::ExportError for Error {
fn exporter_name(&self) -> &'static str {
"otlp"
}

Check warning on line 397 in opentelemetry-otlp/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/lib.rs#L395-L397

Added lines #L395 - L397 were not covered by tests
}

/// The communication protocol to use when exporting data.
#[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
Expand Down
6 changes: 3 additions & 3 deletions opentelemetry-otlp/src/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use async_trait::async_trait;
use std::fmt::Debug;

use opentelemetry::logs::LogResult;
use opentelemetry_sdk::logs::LogResult;

use opentelemetry_sdk::export::logs::LogBatch;

Expand Down Expand Up @@ -62,14 +62,14 @@

#[cfg(feature = "grpc-tonic")]
impl LogExporterBuilder<TonicExporterBuilderSet> {
pub fn build(self) -> Result<LogExporter, opentelemetry::logs::LogError> {
pub fn build(self) -> Result<LogExporter, opentelemetry_sdk::logs::LogError> {

Check warning on line 65 in opentelemetry-otlp/src/logs.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/logs.rs#L65

Added line #L65 was not covered by tests
self.client.0.build_log_exporter()
}
}

#[cfg(any(feature = "http-proto", feature = "http-json"))]
impl LogExporterBuilder<HttpExporterBuilderSet> {
pub fn build(self) -> Result<LogExporter, opentelemetry::logs::LogError> {
pub fn build(self) -> Result<LogExporter, opentelemetry_sdk::logs::LogError> {

Check warning on line 72 in opentelemetry-otlp/src/logs.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/logs.rs#L72

Added line #L72 was not covered by tests
self.client.0.build_log_exporter()
}
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-otlp/src/metric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::NoExporterBuilderSet;

use async_trait::async_trait;
use core::fmt;
use opentelemetry::metrics::MetricResult;
use opentelemetry_sdk::metrics::MetricResult;

use opentelemetry_sdk::metrics::{
data::{ResourceMetrics, Temporality},
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-otlp/tests/integration_test/tests/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

use integration_test_runner::logs_asserter::{read_logs_from_json, LogsAsserter};
use log::{info, Level};
use opentelemetry::logs::LogError;
use opentelemetry::KeyValue;
use opentelemetry_appender_log::OpenTelemetryLogBridge;
use opentelemetry_otlp::{LogExporter, WithExportConfig};
use opentelemetry_sdk::logs::LogError;
use opentelemetry_sdk::logs::LoggerProvider;
use opentelemetry_sdk::{logs as sdklogs, runtime, Resource};
use std::error::Error;
Expand Down
6 changes: 2 additions & 4 deletions opentelemetry-sdk/benches/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ use std::time::SystemTime;

use criterion::{criterion_group, criterion_main, Criterion};

use opentelemetry::logs::{
AnyValue, LogRecord as _, LogResult, Logger as _, LoggerProvider as _, Severity,
};
use opentelemetry::logs::{AnyValue, LogRecord as _, Logger as _, LoggerProvider as _, Severity};
use opentelemetry::trace::Tracer;
use opentelemetry::trace::TracerProvider as _;
use opentelemetry::{InstrumentationScope, Key};
use opentelemetry_sdk::logs::{LogProcessor, LogRecord, Logger, LoggerProvider};
use opentelemetry_sdk::logs::{LogProcessor, LogRecord, LogResult, Logger, LoggerProvider};
use opentelemetry_sdk::trace;
use opentelemetry_sdk::trace::{Sampler, TracerProvider};

Expand Down
3 changes: 2 additions & 1 deletion opentelemetry-sdk/benches/log_exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ use std::time::SystemTime;
use async_trait::async_trait;
use criterion::{criterion_group, criterion_main, Criterion};

use opentelemetry::logs::{LogRecord as _, LogResult, Logger as _, LoggerProvider as _, Severity};
use opentelemetry::logs::{LogRecord as _, Logger as _, LoggerProvider as _, Severity};
use opentelemetry_sdk::logs::LogResult;

use opentelemetry::InstrumentationScope;
use opentelemetry_sdk::export::logs::LogBatch;
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-sdk/benches/log_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ use std::{

use criterion::{criterion_group, criterion_main, Criterion};
use opentelemetry::{
logs::{LogRecord as _, LogResult, Logger as _, LoggerProvider as _, Severity},
logs::{LogRecord as _, Logger as _, LoggerProvider as _, Severity},
InstrumentationScope,
};
use opentelemetry_sdk::logs::{LogProcessor, LogRecord, Logger, LoggerProvider};
use opentelemetry_sdk::logs::{LogProcessor, LogRecord, LogResult, Logger, LoggerProvider};

// Run this benchmark with:
// cargo bench --bench log_processor
Expand Down
6 changes: 3 additions & 3 deletions opentelemetry-sdk/benches/metric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ use std::sync::{Arc, Weak};

use criterion::{criterion_group, criterion_main, Bencher, Criterion};
use opentelemetry::{
metrics::{Counter, Histogram, MeterProvider as _, MetricResult},
metrics::{Counter, Histogram, MeterProvider as _},
Key, KeyValue,
};
use opentelemetry_sdk::{
metrics::{
data::{ResourceMetrics, Temporality},
new_view,
reader::MetricReader,
Aggregation, Instrument, InstrumentKind, ManualReader, Pipeline, SdkMeterProvider, Stream,
View,
Aggregation, Instrument, InstrumentKind, ManualReader, MetricResult, Pipeline,
SdkMeterProvider, Stream, View,
},
Resource,
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
//! Wrapper for error from trace, logs and metrics part of open telemetry.
use std::sync::PoisonError;

#[cfg(feature = "logs")]
use crate::logs::LogError;
#[cfg(feature = "metrics")]
use crate::metrics::MetricError;
use crate::propagation::PropagationError;
use opentelemetry::propagation::PropagationError;
#[cfg(feature = "trace")]
use crate::trace::TraceError;
use opentelemetry::trace::TraceError;

/// Wrapper for error from both tracing and metrics part of open telemetry.
Copy link
Member

Choose a reason for hiding this comment

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

Though this is moving from just one crate to another, I'd like to first confirm that these are indeed required. I understand that it is desirable to return specialized errors from user-facing APIs, but most of the errors here are only for internal logging only. Is there any additional value provided by this?

Copy link
Member

Choose a reason for hiding this comment

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

See #2241 (comment) for prior discussion.
If we don't plan to keep this, then its best to just remove it, instead of moving from api to sdk and then removing it again.
(Note: These are still public APIs in the sdk crate)

#[derive(thiserror::Error, Debug)]
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/export/logs/mod.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//! Log exporters
use crate::logs::LogRecord;
use crate::logs::{LogError, LogResult};
use crate::Resource;
use async_trait::async_trait;
#[cfg(feature = "logs_level_enabled")]
use opentelemetry::logs::Severity;
use opentelemetry::logs::{LogError, LogResult};
use opentelemetry::InstrumentationScope;
use std::fmt::Debug;

Expand Down
6 changes: 5 additions & 1 deletion opentelemetry-sdk/src/export/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@ pub mod logs;
#[cfg_attr(docsrs, doc(cfg(feature = "trace")))]
pub mod trace;

pub use opentelemetry::ExportError;
/// Marker trait for errors returned by exporters
lalitb marked this conversation as resolved.
Show resolved Hide resolved
pub trait ExportError: std::error::Error + Send + Sync + 'static {
/// The name of exporter that returned this error
fn exporter_name(&self) -> &'static str;
}
2 changes: 2 additions & 0 deletions opentelemetry-sdk/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,5 @@ pub mod util;

#[doc(inline)]
pub use resource::Resource;

pub mod error;
Loading
Loading