Skip to content

Commit a043275

Browse files
bnaeckerbnaecker
authored
Move transaction retry timeseries to TOML (#6008)
1 parent 83d2a4c commit a043275

File tree

4 files changed

+338
-30
lines changed

4 files changed

+338
-30
lines changed

‎nexus/db-queries/src/transaction_retry.rs

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,27 +7,15 @@
77
use async_bb8_diesel::AsyncConnection;
88
use chrono::Utc;
99
use diesel::result::Error as DieselError;
10-
use oximeter::{types::Sample, Metric, MetricsError, Target};
10+
use oximeter::{types::Sample, MetricsError};
1111
use rand::{thread_rng, Rng};
1212
use slog::{info, warn, Logger};
1313
use std::sync::{Arc, Mutex};
1414
use std::time::Duration;
1515

16-
// Identifies "which" transaction is retrying
17-
#[derive(Debug, Clone, Target)]
18-
struct DatabaseTransaction {
19-
name: String,
20-
}
21-
22-
// Identifies that a retry has occurred, and track how long
23-
// the transaction took (either since starting, or since the last
24-
// retry failure was recorded).
25-
#[derive(Debug, Clone, Metric)]
26-
struct RetryData {
27-
#[datum]
28-
latency: f64,
29-
attempt: u32,
30-
}
16+
oximeter::use_timeseries!("database-transaction.toml");
17+
use database_transaction::DatabaseTransaction;
18+
use database_transaction::RetryData;
3119

3220
// Collects all transaction retry samples
3321
#[derive(Debug, Default, Clone)]
@@ -156,7 +144,7 @@ impl RetryHelper {
156144

157145
let _ = self.producer.append(
158146
&DatabaseTransaction { name: self.name.into() },
159-
&RetryData { latency, attempt },
147+
&RetryData { datum: latency, attempt },
160148
);
161149

162150
// This backoff is not exponential, but I'm not sure we actually want

‎oximeter/impl/src/schema/codegen.rs

Lines changed: 301 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,140 @@ fn fields_are_copyable<'a>(
8383
fields.all(FieldSchema::is_copyable)
8484
}
8585

86+
/// Return true if the datum type is copyable.
87+
fn datum_type_is_copyable(datum_type: DatumType) -> bool {
88+
match datum_type {
89+
DatumType::Bool
90+
| DatumType::I8
91+
| DatumType::U8
92+
| DatumType::I16
93+
| DatumType::U16
94+
| DatumType::I32
95+
| DatumType::U32
96+
| DatumType::I64
97+
| DatumType::U64
98+
| DatumType::CumulativeI64
99+
| DatumType::CumulativeU64
100+
| DatumType::CumulativeF32
101+
| DatumType::CumulativeF64
102+
| DatumType::F32
103+
| DatumType::F64 => true,
104+
DatumType::String
105+
| DatumType::Bytes
106+
| DatumType::HistogramI8
107+
| DatumType::HistogramU8
108+
| DatumType::HistogramI16
109+
| DatumType::HistogramU16
110+
| DatumType::HistogramI32
111+
| DatumType::HistogramU32
112+
| DatumType::HistogramI64
113+
| DatumType::HistogramU64
114+
| DatumType::HistogramF32
115+
| DatumType::HistogramF64 => false,
116+
}
117+
}
118+
119+
/// Return `true` if values of this datum are partially ordered (can derive
120+
/// `PartialOrd`.)
121+
fn datum_type_is_partially_ordered(datum_type: DatumType) -> bool {
122+
match datum_type {
123+
DatumType::Bool
124+
| DatumType::I8
125+
| DatumType::U8
126+
| DatumType::I16
127+
| DatumType::U16
128+
| DatumType::I32
129+
| DatumType::U32
130+
| DatumType::I64
131+
| DatumType::U64
132+
| DatumType::String
133+
| DatumType::Bytes
134+
| DatumType::CumulativeI64
135+
| DatumType::CumulativeU64
136+
| DatumType::CumulativeF32
137+
| DatumType::CumulativeF64
138+
| DatumType::F32
139+
| DatumType::F64 => true,
140+
DatumType::HistogramI8
141+
| DatumType::HistogramU8
142+
| DatumType::HistogramI16
143+
| DatumType::HistogramU16
144+
| DatumType::HistogramI32
145+
| DatumType::HistogramU32
146+
| DatumType::HistogramI64
147+
| DatumType::HistogramU64
148+
| DatumType::HistogramF32
149+
| DatumType::HistogramF64 => false,
150+
}
151+
}
152+
153+
/// Return `true` if values of this datum are totally ordered (can derive
154+
/// `Ord`.)
155+
fn datum_type_is_totally_ordered(datum_type: DatumType) -> bool {
156+
match datum_type {
157+
DatumType::Bool
158+
| DatumType::I8
159+
| DatumType::U8
160+
| DatumType::I16
161+
| DatumType::U16
162+
| DatumType::I32
163+
| DatumType::U32
164+
| DatumType::I64
165+
| DatumType::U64
166+
| DatumType::String
167+
| DatumType::Bytes
168+
| DatumType::CumulativeI64
169+
| DatumType::CumulativeU64
170+
| DatumType::CumulativeF32
171+
| DatumType::CumulativeF64 => true,
172+
DatumType::F32
173+
| DatumType::F64
174+
| DatumType::HistogramI8
175+
| DatumType::HistogramU8
176+
| DatumType::HistogramI16
177+
| DatumType::HistogramU16
178+
| DatumType::HistogramI32
179+
| DatumType::HistogramU32
180+
| DatumType::HistogramI64
181+
| DatumType::HistogramU64
182+
| DatumType::HistogramF32
183+
| DatumType::HistogramF64 => false,
184+
}
185+
}
186+
187+
/// Return `true` if values of this datum are hashable (can derive `Hash`).
188+
fn datum_type_is_hashable(datum_type: DatumType) -> bool {
189+
match datum_type {
190+
DatumType::Bool
191+
| DatumType::I8
192+
| DatumType::U8
193+
| DatumType::I16
194+
| DatumType::U16
195+
| DatumType::I32
196+
| DatumType::U32
197+
| DatumType::I64
198+
| DatumType::U64
199+
| DatumType::String
200+
| DatumType::Bytes
201+
| DatumType::CumulativeI64
202+
| DatumType::CumulativeU64
203+
| DatumType::CumulativeF32
204+
| DatumType::CumulativeF64 => true,
205+
DatumType::F32
206+
| DatumType::F64
207+
| DatumType::HistogramI8
208+
| DatumType::HistogramU8
209+
| DatumType::HistogramI16
210+
| DatumType::HistogramU16
211+
| DatumType::HistogramI32
212+
| DatumType::HistogramU32
213+
| DatumType::HistogramI64
214+
| DatumType::HistogramU64
215+
| DatumType::HistogramF32
216+
| DatumType::HistogramF64 => false,
217+
}
218+
}
219+
86220
fn compute_extra_derives(
87221
source: FieldSource,
88222
schema: &TimeseriesSchema,
@@ -96,16 +230,25 @@ fn compute_extra_derives(
96230
}
97231
}
98232
FieldSource::Metric => {
99-
if schema.datum_type.is_histogram() {
100-
// No other traits can be derived, since the Histogram<T> won't
101-
// satisfy them.
233+
let mut derives = Vec::new();
234+
if fields_are_copyable(schema.metric_fields())
235+
&& datum_type_is_copyable(schema.datum_type)
236+
{
237+
derives.push(quote! { Copy });
238+
}
239+
if datum_type_is_partially_ordered(schema.datum_type) {
240+
derives.push(quote! { PartialOrd });
241+
if datum_type_is_totally_ordered(schema.datum_type) {
242+
derives.push(quote! { Eq, Ord });
243+
}
244+
}
245+
if datum_type_is_hashable(schema.datum_type) {
246+
derives.push(quote! { Hash })
247+
}
248+
if derives.is_empty() {
102249
quote! {}
103250
} else {
104-
if fields_are_copyable(schema.metric_fields()) {
105-
quote! { #[derive(Copy, Eq, Hash, Ord, PartialOrd)] }
106-
} else {
107-
quote! { #[derive(Eq, Hash, Ord, PartialOrd)] }
108-
}
251+
quote! { #[derive(#(#derives),*)] }
109252
}
110253
}
111254
}
@@ -479,7 +622,7 @@ mod tests {
479622

480623
#[doc = "a metric"]
481624
#[derive(Clone, Debug, PartialEq, ::oximeter::Metric)]
482-
#[derive(Copy, Eq, Hash, Ord, PartialOrd)]
625+
#[derive(Copy, PartialOrd, Eq, Ord, Hash)]
483626
pub struct Bar {
484627
#[doc = "metric field"]
485628
pub f1: ::uuid::Uuid,
@@ -524,12 +667,160 @@ mod tests {
524667

525668
#[doc = "a metric"]
526669
#[derive(Clone, Debug, PartialEq, ::oximeter::Metric)]
527-
#[derive(Copy, Eq, Hash, Ord, PartialOrd)]
670+
#[derive(Copy, PartialOrd, Eq, Ord, Hash)]
528671
pub struct Bar {
529672
pub datum: ::oximeter::types::Cumulative<u64>,
530673
}
531674
};
532675

533676
assert_eq!(tokens.to_string(), expected.to_string());
534677
}
678+
679+
#[test]
680+
fn compute_extra_derives_respects_non_copy_fields() {
681+
// Metric fields are not copy, even though datum is.
682+
let schema = TimeseriesSchema {
683+
timeseries_name: "foo:bar".parse().unwrap(),
684+
description: TimeseriesDescription {
685+
target: "a target".into(),
686+
metric: "a metric".into(),
687+
},
688+
field_schema: BTreeSet::from([FieldSchema {
689+
name: "f0".into(),
690+
field_type: FieldType::String,
691+
source: FieldSource::Metric,
692+
description: "metric field".into(),
693+
}]),
694+
datum_type: DatumType::CumulativeU64,
695+
version: NonZeroU8::new(1).unwrap(),
696+
authz_scope: AuthzScope::Fleet,
697+
units: Units::Bytes,
698+
created: Utc::now(),
699+
};
700+
let tokens = compute_extra_derives(FieldSource::Metric, &schema);
701+
assert_eq!(
702+
tokens.to_string(),
703+
quote! { #[derive(PartialOrd, Eq, Ord, Hash)] }.to_string(),
704+
"Copy should not be derived for a datum type that is copy, \
705+
when the fields themselves are not copy."
706+
);
707+
}
708+
709+
#[test]
710+
fn compute_extra_derives_respects_non_copy_datum_types() {
711+
// Fields are copy, but datum is not.
712+
let schema = TimeseriesSchema {
713+
timeseries_name: "foo:bar".parse().unwrap(),
714+
description: TimeseriesDescription {
715+
target: "a target".into(),
716+
metric: "a metric".into(),
717+
},
718+
field_schema: BTreeSet::from([FieldSchema {
719+
name: "f0".into(),
720+
field_type: FieldType::Uuid,
721+
source: FieldSource::Metric,
722+
description: "metric field".into(),
723+
}]),
724+
datum_type: DatumType::String,
725+
version: NonZeroU8::new(1).unwrap(),
726+
authz_scope: AuthzScope::Fleet,
727+
units: Units::Bytes,
728+
created: Utc::now(),
729+
};
730+
let tokens = compute_extra_derives(FieldSource::Metric, &schema);
731+
assert_eq!(
732+
tokens.to_string(),
733+
quote! { #[derive(PartialOrd, Eq, Ord, Hash)] }.to_string(),
734+
"Copy should not be derived for a datum type that is not copy, \
735+
when the fields themselves are copy."
736+
);
737+
}
738+
739+
#[test]
740+
fn compute_extra_derives_respects_partially_ordered_datum_types() {
741+
// No fields, datum is partially- but not totally-ordered.
742+
let schema = TimeseriesSchema {
743+
timeseries_name: "foo:bar".parse().unwrap(),
744+
description: TimeseriesDescription {
745+
target: "a target".into(),
746+
metric: "a metric".into(),
747+
},
748+
field_schema: BTreeSet::from([FieldSchema {
749+
name: "f0".into(),
750+
field_type: FieldType::Uuid,
751+
source: FieldSource::Target,
752+
description: "target field".into(),
753+
}]),
754+
datum_type: DatumType::F64,
755+
version: NonZeroU8::new(1).unwrap(),
756+
authz_scope: AuthzScope::Fleet,
757+
units: Units::Bytes,
758+
created: Utc::now(),
759+
};
760+
let tokens = compute_extra_derives(FieldSource::Metric, &schema);
761+
assert_eq!(
762+
tokens.to_string(),
763+
quote! { #[derive(Copy, PartialOrd)] }.to_string(),
764+
"Should derive only PartialOrd for a metric type that is \
765+
not totally-ordered."
766+
);
767+
}
768+
769+
#[test]
770+
fn compute_extra_derives_respects_totally_ordered_datum_types() {
771+
// No fields, datum is also totally-ordered
772+
let schema = TimeseriesSchema {
773+
timeseries_name: "foo:bar".parse().unwrap(),
774+
description: TimeseriesDescription {
775+
target: "a target".into(),
776+
metric: "a metric".into(),
777+
},
778+
field_schema: BTreeSet::from([FieldSchema {
779+
name: "f0".into(),
780+
field_type: FieldType::Uuid,
781+
source: FieldSource::Target,
782+
description: "target field".into(),
783+
}]),
784+
datum_type: DatumType::U64,
785+
version: NonZeroU8::new(1).unwrap(),
786+
authz_scope: AuthzScope::Fleet,
787+
units: Units::Bytes,
788+
created: Utc::now(),
789+
};
790+
let tokens = compute_extra_derives(FieldSource::Metric, &schema);
791+
assert_eq!(
792+
tokens.to_string(),
793+
quote! { #[derive(Copy, PartialOrd, Eq, Ord, Hash)] }.to_string(),
794+
"Should derive Ord for a metric type that is totally-ordered."
795+
);
796+
}
797+
798+
#[test]
799+
fn compute_extra_derives_respects_datum_type_with_no_extra_derives() {
800+
// No metric fields, and histograms don't admit any other derives.
801+
let schema = TimeseriesSchema {
802+
timeseries_name: "foo:bar".parse().unwrap(),
803+
description: TimeseriesDescription {
804+
target: "a target".into(),
805+
metric: "a metric".into(),
806+
},
807+
field_schema: BTreeSet::from([FieldSchema {
808+
name: "f0".into(),
809+
field_type: FieldType::String,
810+
source: FieldSource::Target,
811+
description: "target field".into(),
812+
}]),
813+
datum_type: DatumType::HistogramF64,
814+
version: NonZeroU8::new(1).unwrap(),
815+
authz_scope: AuthzScope::Fleet,
816+
units: Units::Bytes,
817+
created: Utc::now(),
818+
};
819+
let tokens = compute_extra_derives(FieldSource::Metric, &schema);
820+
assert!(
821+
tokens.is_empty(),
822+
"A histogram has no extra derives, so a timeseries schema \
823+
with no metric fields should also have no extra derives."
824+
);
825+
}
535826
}

0 commit comments

Comments
 (0)