Make add_histograms use reserved GenericSet names.

dashboard/dashboard/add_histograms.py

 # Copyright 2017 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """URL endpoint for adding new histograms to the dashboard."""
 
 import json
 import sys
 
 from google.appengine.api import taskqueue
 from google.appengine.ext import ndb
 
 from dashboard import add_point_queue
 from dashboard.api import api_request_handler
 from dashboard.common import datastore_hooks
 from dashboard.common import stored_object
 from dashboard.models import histogram
-from tracing.value import histogram as histogram_module
 from tracing.value import histogram_set
 from tracing.value.diagnostics import diagnostic
 from tracing.value.diagnostics import reserved_infos
 
-SUITE_LEVEL_SPARSE_DIAGNOSTIC_NAMES = set(
-    [reserved_infos.ARCHITECTURES.name,
-     reserved_infos.BUG_COMPONENTS.name,
-     reserved_infos.GPUS.name,
-     reserved_infos.MEMORY_AMOUNTS.name,
-     reserved_infos.OS_NAMES.name,
-     reserved_infos.OS_VERSIONS.name,
-     reserved_infos.OWNERS.name,
-     reserved_infos.PRODUCT_VERSIONS.name])
-# TODO(#3507): Make BuildbotInfo into GenericSet diagnostics and remove all
-# logic regarding picking diagnostics by type.
-SUITE_LEVEL_SPARSE_DIAGNOSTIC_TYPES = set(
-    [histogram_module.BuildbotInfo])
-HISTOGRAM_LEVEL_SPARSE_DIAGNOSTIC_TYPES = set(
-    [histogram_module.TelemetryInfo])
-SPARSE_DIAGNOSTIC_TYPES = SUITE_LEVEL_SPARSE_DIAGNOSTIC_TYPES.union(
-    HISTOGRAM_LEVEL_SPARSE_DIAGNOSTIC_TYPES)
+SUITE_LEVEL_SPARSE_DIAGNOSTIC_NAMES = set([
+    reserved_infos.ARCHITECTURES.name,
+    reserved_infos.BENCHMARKS.name,
+    reserved_infos.BOTS.name,
+    reserved_infos.BUG_COMPONENTS.name,
+    reserved_infos.GPUS.name,
+    reserved_infos.MASTERS.name,
+    reserved_infos.MEMORY_AMOUNTS.name,
+    reserved_infos.OS_NAMES.name,
+    reserved_infos.OS_VERSIONS.name,
+    reserved_infos.OWNERS.name,
+    reserved_infos.PRODUCT_VERSIONS.name,
+    reserved_infos.TAG_MAP.name,
+])
+
+HISTOGRAM_LEVEL_SPARSE_DIAGNOSTIC_NAMES = set([
+    reserved_infos.GPUS.name,

[eakuefner, 2017/08/18 23:32:21]

GPUs we may have to double-check -- I think the information may be the same, but
let's confirm that the ordering is deterministic on a per-device basis. Could
you file a bug under the histogram pipeline project to look into this?

+    reserved_infos.MEMORY_AMOUNTS.name,
+    reserved_infos.PRODUCT_VERSIONS.name,
+    reserved_infos.RELATED_NAMES.name,
+    reserved_infos.STORIES.name,
+    reserved_infos.STORYSET_REPEATS.name,
+    reserved_infos.STORY_TAGS.name,
+])
+
+SPARSE_DIAGNOSTIC_NAMES = SUITE_LEVEL_SPARSE_DIAGNOSTIC_NAMES.union(
+    HISTOGRAM_LEVEL_SPARSE_DIAGNOSTIC_NAMES)
 
 
 TASK_QUEUE_NAME = 'histograms-queue'
 
 
 def _CheckRequest(condition, msg):
   if not condition:
     raise api_request_handler.BadRequestError(msg)
 
 
@@ skipping 34 matching lines @@
   for hist in histograms:
     for name, diag in hist.diagnostics.iteritems():
       if name in SUITE_LEVEL_SPARSE_DIAGNOSTIC_NAMES:
         if diagnostic_names_added.get(name) is None:
           diagnostic_names_added[name] = diag.guid
 
         if diagnostic_names_added.get(name) != diag.guid:
           raise ValueError(
               name + ' diagnostics must be the same for all histograms')
 
-      if (name in SUITE_LEVEL_SPARSE_DIAGNOSTIC_NAMES or
-          type(diag) in SUITE_LEVEL_SPARSE_DIAGNOSTIC_TYPES):
+      if name in SUITE_LEVEL_SPARSE_DIAGNOSTIC_NAMES:
         suite_level_sparse_diagnostic_entities.append(
             histogram.SparseDiagnostic(
                 id=diag.guid, data=diag.AsDict(), test=suite_key,
                 start_revision=revision, end_revision=sys.maxint, name=name))
 
   # TODO(eakuefner): Refactor master/bot computation to happen above this line
   # so that we can replace with a DiagnosticRef rather than a full diagnostic.
   new_guids_to_old_diagnostics = DeduplicateAndPut(
       suite_level_sparse_diagnostic_entities, suite_key, revision)
   for new_guid, old_diagnostic in new_guids_to_old_diagnostics.iteritems():
@@ skipping 62 matching lines @@
 
 
 def _IsDifferent(diagnostic_a, diagnostic_b):
   return (diagnostic.Diagnostic.FromDict(diagnostic_a) !=
           diagnostic.Diagnostic.FromDict(diagnostic_b))
 
 
 def FindHistogramLevelSparseDiagnostics(guid, histograms):
   hist = histograms.LookupHistogram(guid)
   diagnostics = []
-  for diag in hist.diagnostics.itervalues():
-    if type(diag) in HISTOGRAM_LEVEL_SPARSE_DIAGNOSTIC_TYPES:
+  for name, diag in hist.diagnostics.iteritems():
+    if name in HISTOGRAM_LEVEL_SPARSE_DIAGNOSTIC_NAMES:
       diagnostics.append(diag)
   return diagnostics
 
 
 def GetSuiteKey(histograms):
   assert len(histograms) > 0
   # TODO(eakuefner): Refactor this to coalesce the boilerplate (note that this
   # is all also being done in add_histograms_queue's post handler)
   master, bot, benchmark = _GetMasterBotBenchmarkFromHistogram(
       histograms.GetFirstHistogram())
   bot_whitelist = stored_object.Get(add_point_queue.BOT_WHITELIST_KEY)
   internal_only = add_point_queue.BotInternalOnly(bot, bot_whitelist)
   return add_point_queue.GetOrCreateAncestors(
       master, bot, benchmark, internal_only).key
 
 
 def ComputeTestPath(guid, histograms):
   hist = histograms.LookupHistogram(guid)
   suite_path = '%s/%s/%s' % _GetMasterBotBenchmarkFromHistogram(hist)
-  telemetry_info = hist.diagnostics[reserved_infos.TELEMETRY.name]
-  story_display_name = telemetry_info.story_display_name
-
   path = '%s/%s' % (suite_path, hist.name)
 
-  if story_display_name != '':
-    path += '/%s' % story_display_name
+  story_name = hist.diagnostics.get(reserved_infos.STORIES.name)
+  if story_name and len(story_name) == 1:
+    path += '/' + list(story_name)[0]

[eakuefner, 2017/08/18 23:32:21]

Can we have GenericSet.GetOnlyElement?

 
   return path
 
 
 def _GetMasterBotBenchmarkFromHistogram(hist):
-  _CheckRequest(reserved_infos.BUILDBOT.name in hist.diagnostics,
-                'Histograms must have BuildbotInfo attached')
-  buildbot_info = hist.diagnostics[reserved_infos.BUILDBOT.name]
   _CheckRequest(
-      reserved_infos.TELEMETRY.name in hist.diagnostics,
-      'Histograms must have TelemetryInfo attached')
-  telemetry_info = hist.diagnostics[reserved_infos.TELEMETRY.name]
+      reserved_infos.MASTERS.name in hist.diagnostics,
+      'Histograms must have "%s" diagnostic' % reserved_infos.MASTERS.name)
+  master = hist.diagnostics[reserved_infos.MASTERS.name]
+  _CheckRequest(
+      len(master) == 1,
+      'Histograms must have exactly 1 "%s"' % reserved_infos.MASTERS.name)
+  master = list(master)[0]
 
-  master = buildbot_info.display_master_name
-  bot = buildbot_info.display_bot_name
-  benchmark = telemetry_info.benchmark_name
+  _CheckRequest(
+      reserved_infos.BOTS.name in hist.diagnostics,
+      'Histograms must have "%s" diagnostic' % reserved_infos.BOTS.name)
+  bot = hist.diagnostics[reserved_infos.BOTS.name]
+  _CheckRequest(
+      len(bot) == 1,
+      'Histograms must have exactly 1 "%s"' % reserved_infos.BOTS.name)

[eakuefner, 2017/08/18 23:32:21]

We may need to revisit this in the per-story sharding world.

+  bot = list(bot)[0]
+
+  _CheckRequest(
+      reserved_infos.BENCHMARKS.name in hist.diagnostics,
+      'Histograms must have "%s" diagnostic' % reserved_infos.BENCHMARKS.name)
+  benchmark = hist.diagnostics[reserved_infos.BENCHMARKS.name]
+  _CheckRequest(
+      len(benchmark) == 1,
+      'Histograms must have exactly 1 "%s"' % reserved_infos.BENCHMARKS.name)
+  benchmark = list(benchmark)[0]
 
   return master, bot, benchmark
 
 
 def ComputeRevision(histograms):
   _CheckRequest(len(histograms) > 0, 'Must upload at least one histogram')
   diagnostics = histograms.GetFirstHistogram().diagnostics
   _CheckRequest(reserved_infos.CHROMIUM_COMMIT_POSITIONS.name in diagnostics,
                 'Histograms must have Chromium commit position attached')
   chromium_commit_position = list(diagnostics[
       reserved_infos.CHROMIUM_COMMIT_POSITIONS.name])
 
   _CheckRequest(len(chromium_commit_position) == 1,
                 'Chromium commit position must have 1 value')
 
   # TODO(eakuefner): Allow users to specify other types of revisions to be used
   # for computing revisions of dashboard points. See
   # https://github.com/catapult-project/catapult/issues/3623.
   return chromium_commit_position[0]
 
 
 def InlineDenseSharedDiagnostics(histograms):
   # TODO(eakuefner): Delete inlined diagnostics from the set
   for hist in histograms:
     diagnostics = hist.diagnostics
     for name, diag in diagnostics.iteritems():
-      if (type(diag) not in SPARSE_DIAGNOSTIC_TYPES and
-          name not in SUITE_LEVEL_SPARSE_DIAGNOSTIC_NAMES):
+      if name not in SPARSE_DIAGNOSTIC_NAMES:
         diag.Inline()