monitoringdashboard: round trip tests #2191
Conversation
justinsb
force-pushed
the
monitoringdashboard_cleanup_roundtrips
branch
from
ffeb042 to
277273f
Compare
Create a script to run our round-trip tests, also stop excluding some fields we have added support for, and normalize more values to be valid (e.g. alignment_period only supports whole seconds)
justinsb
force-pushed
the
monitoringdashboard_cleanup_roundtrips
branch
from
277273f to
4f47814
Compare
| @@ -142,8 +142,16 @@ func Duration_FromProto(mapCtx *MapContext, in *durationpb.Duration) *string { | |||
| return nil | |||
| } | |||
|
|
|||
| d := in.AsDuration() | |||
| s := fmt.Sprintf("%vs", d.Seconds()) | |||
| // We want to report the duration without truncation (do don't want to map via float64) | |||
There was a problem hiding this comment.
The issue we're fixing is that if translation to float64 products a number that is too big (uses exponents) or too small (nanosecond precision), it wasn't round-tripping.
In theory this is in the production code, in practice it doesn't make a difference for reasonable values.
|
|
||
| unimplementedFields.Insert(widgetPath + ".id") | ||
| unimplementedFields.Insert(widgetPath + ".time_series_table.data_sets[].time_series_query.time_series_filter.statistical_time_series_filter") | ||
| unimplementedFields.Insert(widgetPath + ".time_series_table.data_sets[].time_series_query.time_series_filter.pick_time_series_filter.interval") |
There was a problem hiding this comment.
I haven't found any examples for statisticial_time_series_filter or interval, so I'm not sure if we should try to support it yet.
| // This one might be a bug? | ||
| unimplementedFields.Insert(widgetPath + ".xy_chart.data_sets[].min_alignment_period.nanos") | ||
|
|
||
| unimplementedFields.Insert(widgetPath + ".scorecard.thresholds[].target_axis") | ||
| unimplementedFields.Insert(widgetPath + ".scorecard.blank_view") |
There was a problem hiding this comment.
Blank view I think we probably should support, but I think it can go into 1.121 if need be.
| @@ -35,6 +35,7 @@ output fields from GCP APIs are in `status.observedState.*` | |||
| * Added `pieChart` widgets. | |||
| * Added `sectionHeader` widgets. | |||
| * Added `singleViewGroup` widgets. | |||
| * Added `timeSeriesTable` widgets. | |||
There was a problem hiding this comment.
I want to merge the PR without this change. Do you mind reverting?
There was a problem hiding this comment.
Okay, just checked that the timeSeriesTable field has been checked in. Thank you for updating this note!
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuwenma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
google-oss-prow
bot
merged commit c9317f5
into
GoogleCloudPlatform:master
Create a script to run our round-trip tests, also stop excluding some fields we
have added support for, and normalize more values to be valid
(e.g. alignment_period only supports whole seconds)