1271986 - Skip extended type checks in histogram_tools.py for "keyed" property as well

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Georg Fritzsche [:gfritzsche]]

Similar to the properties handled by bug 1263189, but for the "keyed" property:

- Bug 1
  File "/home/hadoop/anaconda2/lib/python2.7/site-packages/mozaggregator/db.py", line 63, in <lambda>
    map(lambda x: (x[0][:4], _aggregate_to_sql(x))).\
  File "/home/hadoop/anaconda2/lib/python2.7/site-packages/mozaggregator/db.py", line 120, in _aggregate_to_sql
    histogram = _get_complete_histogram(channel, metric, payload["histogram"]) + [payload["sum"], payload["count"]]
  File "/home/hadoop/anaconda2/lib/python2.7/site-packages/mozaggregator/db.py", line 93, in _get_complete_histogram
    histogram = Histogram(metric, {"values": values}, revision=revision).get_value(autocast=False).values
  File "/home/hadoop/anaconda2/lib/python2.7/site-packages/moztelemetry/histogram.py", line 117, in __init__
    self.definition = histogram_tools.Histogram(name, histograms_definition[proper_name])
  File "/home/hadoop/anaconda2/lib/python2.7/site-packages/moztelemetry/histogram_tools.py", line 102, in __init__
    self.verify_attributes(name, definition)
  File "/home/hadoop/anaconda2/lib/python2.7/site-packages/moztelemetry/histogram_tools.py", line 225, in verify_attributes
    self.check_field_types(name, definition)
  File "/home/hadoop/anaconda2/lib/python2.7/site-packages/moztelemetry/histogram_tools.py", line 300, in check_field_types
    'should be {2}').format(key, name, type_name)
ValueError: value for key "keyed" in Histogram "ADDON_SHIM_USAGE" should be bool

Comment 1

[Georg Fritzsche [:gfritzsche]]

Attachment: Allow for Histogram.json entries with "keyed":"true" when running histogram_tools.py in the pipeline

Comment 2

[Alessio Placitelli [:Dexter]]

Comment on attachment 8751287 [details] [diff] [review]
Allow for Histogram.json entries with "keyed":"true" when running histogram_tools.py in the pipeline

Review of attachment 8751287 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/telemetry/histogram_tools.py
@@ +293,5 @@
>                      return v
>              for key in [k for k in coerce_fields if k in definition]:
>                  definition[key] = try_to_coerce_to_number(definition[key])
> +            # This handles old "keyed":"true" definitions (bug 1271986).
> +            if definition.get("keyed", None) == "true":

Do we need to care about case sensitivity here?

Comment 3

[Georg Fritzsche [:gfritzsche]]

(In reply to Alessio Placitelli [:Dexter] from comment #2)
> Comment on attachment 8751287 [details] [diff] [review]
> Allow for Histogram.json entries with "keyed":"true" when running
> histogram_tools.py in the pipeline
> 
> Review of attachment 8751287 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/telemetry/histogram_tools.py
> @@ +293,5 @@
> >                      return v
> >              for key in [k for k in coerce_fields if k in definition]:
> >                  definition[key] = try_to_coerce_to_number(definition[key])
> > +            # This handles old "keyed":"true" definitions (bug 1271986).
> > +            if definition.get("keyed", None) == "true":
> 
> Do we need to care about case sensitivity here?

No, this is just compatibility for the old definitions.

Comment 4

[Alessio Placitelli [:Dexter]]

Comment on attachment 8751287 [details] [diff] [review]
Allow for Histogram.json entries with "keyed":"true" when running histogram_tools.py in the pipeline

Review of attachment 8751287 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/1e5b69b0387b

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1e5b69b0387b