Skip to content

Commit

Permalink
Revert "Remove API for Creating Histograms with signed integers. (ope…
Browse files Browse the repository at this point in the history
…n-telemetry#1371)"

This reverts commit c06c04b.
  • Loading branch information
jakubadamw committed Jan 31, 2024
1 parent 864621c commit f25e248
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 11 deletions.
2 changes: 1 addition & 1 deletion opentelemetry-prometheus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
//! .with_description("Counts things")
//! .init();
//! let histogram = meter
//! .u64_histogram("a.histogram")
//! .i64_histogram("a.histogram")
//! .with_description("Records values")
//! .init();
//!
Expand Down
14 changes: 7 additions & 7 deletions opentelemetry-prometheus/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,15 +515,15 @@ fn duplicate_metrics() {
name: "no_conflict_two_histograms",
record_metrics: Box::new(|meter_a, meter_b| {
let foo_a = meter_a
.u64_histogram("foo")
.i64_histogram("foo")
.with_unit(Unit::new("By"))
.with_description("meter histogram foo")
.init();

foo_a.record(100, &[KeyValue::new("A", "B")]);

let foo_b = meter_b
.u64_histogram("foo")
.i64_histogram("foo")
.with_unit(Unit::new("By"))
.with_description("meter histogram foo")
.init();
Expand Down Expand Up @@ -587,15 +587,15 @@ fn duplicate_metrics() {
name: "conflict_help_two_histograms",
record_metrics: Box::new(|meter_a, meter_b| {
let bar_a = meter_a
.u64_histogram("bar")
.i64_histogram("bar")
.with_unit(Unit::new("By"))
.with_description("meter a bar")
.init();

bar_a.record(100, &[KeyValue::new("A", "B")]);

let bar_b = meter_b
.u64_histogram("bar")
.i64_histogram("bar")
.with_unit(Unit::new("By"))
.with_description("meter b bar")
.init();
Expand Down Expand Up @@ -658,15 +658,15 @@ fn duplicate_metrics() {
name: "conflict_unit_two_histograms",
record_metrics: Box::new(|meter_a, meter_b| {
let bar_a = meter_a
.u64_histogram("bar")
.i64_histogram("bar")
.with_unit(Unit::new("By"))
.with_description("meter histogram bar")
.init();

bar_a.record(100, &[KeyValue::new("A", "B")]);

let bar_b = meter_b
.u64_histogram("bar")
.i64_histogram("bar")
.with_unit(Unit::new("ms"))
.with_description("meter histogram bar")
.init();
Expand Down Expand Up @@ -715,7 +715,7 @@ fn duplicate_metrics() {
foo_a.add(100, &[KeyValue::new("A", "B")]);

let foo_histogram_a = meter_a
.u64_histogram("foo")
.i64_histogram("foo")
.with_unit(Unit::new("By"))
.with_description("meter histogram foo")
.init();
Expand Down
9 changes: 7 additions & 2 deletions opentelemetry-sdk/benches/metric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ fn counters(c: &mut Criterion) {

const MAX_BOUND: usize = 100000;

fn bench_histogram(bound_count: usize) -> (SharedReader, Histogram<u64>) {
fn bench_histogram(bound_count: usize) -> (SharedReader, Histogram<i64>) {
let mut bounds = vec![0; bound_count];
#[allow(clippy::needless_range_loop)]
for i in 0..bounds.len() {
Expand All @@ -380,7 +380,7 @@ fn bench_histogram(bound_count: usize) -> (SharedReader, Histogram<u64>) {
}
let mtr = builder.build().meter("test_meter");
let hist = mtr
.u64_histogram(format!("histogram_{}", bound_count))
.i64_histogram(format!("histogram_{}", bound_count))
.init();

(r, hist)
Expand Down Expand Up @@ -423,8 +423,13 @@ fn benchmark_collect_histogram(b: &mut Bencher, n: usize) {
.meter("sdk/metric/bench/histogram");

for i in 0..n {
<<<<<<< HEAD
let h = mtr.u64_histogram(format!("fake_data_{i}")).init();
h.record(1, AttributeSet::default());
=======
let h = mtr.i64_histogram(format!("fake_data_{i}")).init();
h.record(1, &[]);
>>>>>>> parent of c06c04b (Remove API for Creating Histograms with signed integers. (#1371))
}

let mut rm = ResourceMetrics {
Expand Down
24 changes: 24 additions & 0 deletions opentelemetry-sdk/src/metrics/meter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,24 @@ impl InstrumentProvider for SdkMeter {
.map(|i| Histogram::new(Arc::new(i)))
}

fn i64_histogram(
&self,
name: Cow<'static, str>,
description: Option<Cow<'static, str>>,
unit: Option<Unit>,
) -> Result<Histogram<i64>> {
validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?;
let p = InstrumentResolver::new(self, &self.i64_resolver);

p.lookup(
InstrumentKind::Histogram,
name,
description,
unit.unwrap_or_default(),
)
.map(|i| Histogram::new(Arc::new(i)))
}

fn register_callback(
&self,
insts: &[Arc<dyn Any>],
Expand Down Expand Up @@ -855,6 +873,7 @@ mod tests {
);
assert(meter.f64_histogram(name.into(), None, None).map(|_| ()));
assert(meter.u64_histogram(name.into(), None, None).map(|_| ()));
assert(meter.i64_histogram(name.into(), None, None).map(|_| ()));
}

// (unit, expected error)
Expand Down Expand Up @@ -944,6 +963,11 @@ mod tests {
.u64_histogram("test".into(), None, unit.clone())
.map(|_| ()),
);
assert(
meter
.i64_histogram("test".into(), None, unit.clone())
.map(|_| ()),
);
}
}
}
2 changes: 1 addition & 1 deletion opentelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ gains, and avoids `IndexMap` dependency. This affects `body` and `attributes` of

### Removed

- Removed `OrderMap` type as there was no requirement to use this over regular
Removed `OrderMap` type as there was no requirement to use this over regular
`HashMap`.
[#1353](https://github.com/open-telemetry/opentelemetry-rust/pull/1353)
- Remove API for Creating Histograms with signed integers. [#1371](https://github.com/open-telemetry/opentelemetry-rust/pull/1371)
Expand Down
12 changes: 12 additions & 0 deletions opentelemetry/src/metrics/instruments/histogram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,15 @@ impl TryFrom<InstrumentBuilder<'_, Histogram<u64>>> for Histogram<u64> {
)
}
}

impl TryFrom<InstrumentBuilder<'_, Histogram<i64>>> for Histogram<i64> {
type Error = MetricsError;

fn try_from(builder: InstrumentBuilder<'_, Histogram<i64>>) -> Result<Self, Self::Error> {
builder.meter.instrument_provider.i64_histogram(
builder.name,
builder.description,
builder.unit,
)
}
}
21 changes: 21 additions & 0 deletions opentelemetry/src/metrics/meter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,19 @@ pub trait MeterProvider {
/// // Record measurements using the histogram instrument record()
/// f64_histogram.record(10.5, attributes.clone());
///
/// // i64 histogram
/// let i64_histogram = meter.i64_histogram("my_i64_histogram").init();
///
/// // Record measurements using the histogram instrument record()
/// i64_histogram.record(
/// 1,
/// [
/// KeyValue::new("mykey1", "myvalue1"),
/// KeyValue::new("mykey2", "myvalue2"),
/// ]
/// .as_ref(),
/// );
///
/// // u64 histogram
/// let u64_histogram = meter.u64_histogram("my_u64_histogram").init();
///
Expand Down Expand Up @@ -335,6 +348,14 @@ impl Meter {
InstrumentBuilder::new(self, name.into())
}

/// creates an instrument builder for recording a distribution of values.
pub fn i64_histogram(
&self,
name: impl Into<Cow<'static, str>>,
) -> InstrumentBuilder<'_, Histogram<i64>> {
InstrumentBuilder::new(self, name.into())
}

/// Registers a callback to be called during the collection of a measurement
/// cycle.
///
Expand Down
10 changes: 10 additions & 0 deletions opentelemetry/src/metrics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,16 @@ pub trait InstrumentProvider {
Ok(Histogram::new(Arc::new(noop::NoopSyncInstrument::new())))
}

/// creates an instrument for recording a distribution of values.
fn i64_histogram(
&self,
_name: Cow<'static, str>,
_description: Option<Cow<'static, str>>,
_unit: Option<Unit>,
) -> Result<Histogram<i64>> {
Ok(Histogram::new(Arc::new(noop::NoopSyncInstrument::new())))
}

/// Captures the function that will be called during data collection.
///
/// It is only valid to call `observe` within the scope of the passed function.
Expand Down

0 comments on commit f25e248

Please sign in to comment.