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

Remove API for Creating Histograms with signed integers. #1371

Merged
merged 6 commits into from Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion opentelemetry-prometheus/src/lib.rs
Expand Up @@ -27,7 +27,7 @@
//! .with_description("Counts things")
//! .init();
//! let histogram = meter
//! .i64_histogram("a.histogram")
//! .u64_histogram("a.histogram")
//! .with_description("Records values")
//! .init();
//!
Expand Down
14 changes: 7 additions & 7 deletions opentelemetry-prometheus/tests/integration_test.rs
Expand Up @@ -492,15 +492,15 @@ fn duplicate_metrics() {
name: "no_conflict_two_histograms",
record_metrics: Box::new(|meter_a, meter_b| {
let foo_a = meter_a
.i64_histogram("foo")
.u64_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
.i64_histogram("foo")
.u64_histogram("foo")
.with_unit(Unit::new("By"))
.with_description("meter histogram foo")
.init();
Expand Down Expand Up @@ -564,15 +564,15 @@ fn duplicate_metrics() {
name: "conflict_help_two_histograms",
record_metrics: Box::new(|meter_a, meter_b| {
let bar_a = meter_a
.i64_histogram("bar")
.u64_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
.i64_histogram("bar")
.u64_histogram("bar")
.with_unit(Unit::new("By"))
.with_description("meter b bar")
.init();
Expand Down Expand Up @@ -635,15 +635,15 @@ fn duplicate_metrics() {
name: "conflict_unit_two_histograms",
record_metrics: Box::new(|meter_a, meter_b| {
let bar_a = meter_a
.i64_histogram("bar")
.u64_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
.i64_histogram("bar")
.u64_histogram("bar")
.with_unit(Unit::new("ms"))
.with_description("meter histogram bar")
.init();
Expand Down Expand Up @@ -692,7 +692,7 @@ fn duplicate_metrics() {
foo_a.add(100, &[KeyValue::new("A", "B")]);

let foo_histogram_a = meter_a
.i64_histogram("foo")
.u64_histogram("foo")
.with_unit(Unit::new("By"))
.with_description("meter histogram foo")
.init();
Expand Down
8 changes: 4 additions & 4 deletions opentelemetry-sdk/benches/metric.rs
Expand Up @@ -349,7 +349,7 @@ fn counters(c: &mut Criterion) {

const MAX_BOUND: usize = 100000;

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

(r, hist)
Expand All @@ -393,7 +393,7 @@ fn histograms(c: &mut Criterion) {
format!("V,{},{},{}", bound_size, attr_size, i),
))
}
let value: i64 = rng.gen_range(0..MAX_BOUND).try_into().unwrap();
let value: u64 = rng.gen_range(0..MAX_BOUND).try_into().unwrap();
group.bench_function(
format!("Record{}Attrs{}bounds", attr_size, bound_size),
|b| b.iter(|| hist.record(value, &attributes)),
Expand All @@ -414,7 +414,7 @@ fn benchmark_collect_histogram(b: &mut Bencher, n: usize) {
.meter("sdk/metric/bench/histogram");

for i in 0..n {
let h = mtr.i64_histogram(format!("fake_data_{i}")).init();
let h = mtr.u64_histogram(format!("fake_data_{i}")).init();
h.record(1, &[]);
}

Expand Down
24 changes: 0 additions & 24 deletions opentelemetry-sdk/src/metrics/meter.rs
Expand Up @@ -444,24 +444,6 @@ 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 @@ -819,7 +801,6 @@ 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 @@ -909,11 +890,6 @@ mod tests {
.u64_histogram("test".into(), None, unit.clone())
.map(|_| ()),
);
assert(
meter
.i64_histogram("test".into(), None, unit.clone())
.map(|_| ()),
);
}
}
}
3 changes: 2 additions & 1 deletion opentelemetry/CHANGELOG.md
Expand Up @@ -13,9 +13,10 @@ 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)

## [v0.21.0](https://github.com/open-telemetry/opentelemetry-rust/compare/v0.20.0...v0.21.0)

Expand Down
12 changes: 0 additions & 12 deletions opentelemetry/src/metrics/instruments/histogram.rs
Expand Up @@ -60,15 +60,3 @@ 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: 0 additions & 21 deletions opentelemetry/src/metrics/meter.rs
Expand Up @@ -241,19 +241,6 @@ pub trait MeterProvider {
/// .as_ref(),
/// );
///
/// // 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 @@ -386,14 +373,6 @@ 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: 0 additions & 10 deletions opentelemetry/src/metrics/mod.rs
Expand Up @@ -238,16 +238,6 @@ 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