New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 684974 link

Starred by 2 users
Status: Fixed
Owner:
█ dsansome@chromium.org
Last visit > 30 days ago
Closed: Mar 2017
Cc:
vadimsh@chromium.org
dpranke@chromium.org
sergeybe...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Python ts_mon: require fields to be defined with the metric

Project Member Reported by dsansome@chromium.org, Jan 25 2017 
Currently field names and values are provided when setting or updating metrics.  It's too easy to use different sets of field names for the same metric, which is an error and causes metrics to be silently dropped.

We should change the API to make this impossible.
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 15 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/15a4affd130d0e270ed520b72c4b8c24c90dbe27

commit 15a4affd130d0e270ed520b72c4b8c24c90dbe27
Author: David Sansome <dsansome@chromium.org>
Date: Wed Feb 15 04:16:53 2017

Define field names and types at metric creation time.

This prevents a common problem where users will set different sets of fields on
the same metric, causing their streams to be rejected by the backend.

Also make the description mandatory while we're breaking the API.

This is a major API change, so I'll separately update all ts_mon users when we
roll infra_libs in infra, and again when we roll infra in infra_internal.

BUG= 684974 

Change-Id: I3dcd524bf7b99ddecbb78b34cbf59a23849cc80f
Reviewed-on: https://chromium-review.googlesource.com/432120
Commit-Queue: Dave Sansome <dsansome@chromium.org>
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Sergey Berezin <sergeyberezin@chromium.org>

[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/setup.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/__init__.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/test/metrics_test.expected/MetricTest.test_serialize_with_units.json
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/test/metrics_test.expected/MetricTest.test_serialize.json
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/http_metrics.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/metrics.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/test/interface_test.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/test/metrics_test.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/logs/logs.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/test/metric_store_test.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/test/errors_test.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/interface.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/standard_metrics.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/errors.py
[modify] https://crrev.com/15a4affd130d0e270ed520b72c4b8c24c90dbe27/packages/infra_libs/infra_libs/ts_mon/common/test/metrics_test.expected/MetricTest.test_serialize_with_description.json
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 15 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/07fe60f4876317a695098dfcc23c0d7e5408b4ed

commit 07fe60f4876317a695098dfcc23c0d7e5408b4ed
Author: David Sansome <dsansome@chromium.org>
Date: Wed Feb 15 05:23:41 2017

Add field_specs to all metrics in infra/infra.

BUG= 684974 

Change-Id: Ibd9097e56e3e99defbf650fd31347cd56fcec77b
Reviewed-on: https://chromium-review.googlesource.com/442407
Commit-Queue: Dave Sansome <dsansome@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/infra/tools/send_ts_mon_values/common.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/findit/waterfall/monitoring.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/infra/services/mastermon/test/pollers_test.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/chromium_try_flakes/handlers/flake_issues.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/infra/services/mastermon/test/monitor_test.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/infra/services/gsubtreed/__main__.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/chromium_rietveld/codereview/views.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine_module/gae_ts_mon/shared.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine_module/gae_ts_mon/test/config_test.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/monorail/services/client_config_svc.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/infra/services/service_manager/service_thread.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/monorail/services/spam_svc.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine_module/gae_ts_mon/__init__.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/monorail/framework/ratelimiter.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/infra/tools/send_monitoring_event/__main__.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/test_results/appengine_module/test_results/handlers/monitoring.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/monorail/services/api_svc_v1.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/monorail/framework/sql.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/infra/services/mastermon/monitor.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/cr-buildbucket/metrics.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/monorail/services/issue_svc.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine_module/gae_ts_mon/README.md
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine_module/gae_ts_mon/test/shared_test.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/infra/tools/master_manager/__main__.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/bootstrap/deps.pyl
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/findit/crash/monitoring.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/infra/services/mastermon/pollers.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/monorail/framework/clientmon.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/appengine/chromium_try_flakes/status/cq_status.py
[modify] https://crrev.com/07fe60f4876317a695098dfcc23c0d7e5408b4ed/infra/libs/service_utils/outer_loop.py
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 21 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4

commit 9ba306ed3b72c50f41f7a16e94e744b07bb46dc4
Author: dsansome <dsansome@chromium.org>
Date: Tue Feb 21 00:00:25 2017

Roll infra_libs and gae_ts_mon in luci-py, and add field_specs to all metrics

BUG= 684974 

Review-Url: https://codereview.chromium.org/2709453002

[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/appengine/gce-backend/metrics.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/appengine/machine_provider/metrics.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/appengine/swarming/ts_mon_metrics.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/appengine/third_party/gae_ts_mon/README.md
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/appengine/third_party/gae_ts_mon/__init__.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/appengine/third_party/gae_ts_mon/config.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/appengine/third_party/gae_ts_mon/shared.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/event_mon/protos/goma_stats_pb2.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/httplib2_utils.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/logs/logs.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/ts_mon/__init__.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/ts_mon/common/errors.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/ts_mon/common/http_metrics.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/ts_mon/common/interface.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/ts_mon/common/metrics.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/ts_mon/common/monitors.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/ts_mon/common/standard_metrics.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/ts_mon/config.py
[modify] https://crrev.com/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4/client/third_party/infra_libs/utils.py
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 21 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/1f350205379c09582b4c5950e91a0b8a031ecf28

commit 1f350205379c09582b4c5950e91a0b8a031ecf28
Author: David Sansome <dsansome@chromium.org>
Date: Tue Feb 21 05:21:37 2017

Roll luci-py in infra to pick up a new infra_libs from infra.  We need to go deeper.

BUG= 684974 
TBR=vadimsh

Change-Id: I869a0b636961dd0b354ed67d1ca4422680afda4c
Reviewed-on: https://chromium-review.googlesource.com/445476
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Dave Sansome <dsansome@chromium.org>
Commit-Queue: Dave Sansome <dsansome@chromium.org>

[modify] https://crrev.com/1f350205379c09582b4c5950e91a0b8a031ecf28/DEPS
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 21 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/f608998325f9c7eca8a7b418b2ea71fab36d07e8

commit f608998325f9c7eca8a7b418b2ea71fab36d07e8
Author: maruel <maruel@chromium.org>
Date: Tue Feb 21 14:38:57 2017

Revert of Add field_specs to all metrics in luci-py (patchset #2 id:20001 of https://codereview.chromium.org/2709453002/ )

Reason for revert:
Reverting because rolling this version would break the instances.

Original issue's description:
> Roll infra_libs and gae_ts_mon in luci-py, and add field_specs to all metrics
>
> BUG= 684974 
>
> Review-Url: https://codereview.chromium.org/2709453002
> Committed: https://github.com/luci/luci-py/commit/9ba306ed3b72c50f41f7a16e94e744b07bb46dc4

TBR=vadimsh@chromium.org,sergeyberezin@chromium.org,dsansome@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 684974 

Review-Url: https://codereview.chromium.org/2708113002

[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/appengine/gce-backend/metrics.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/appengine/machine_provider/metrics.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/appengine/swarming/ts_mon_metrics.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/appengine/third_party/gae_ts_mon/README.md
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/appengine/third_party/gae_ts_mon/__init__.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/appengine/third_party/gae_ts_mon/config.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/appengine/third_party/gae_ts_mon/shared.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/event_mon/protos/goma_stats_pb2.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/httplib2_utils.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/logs/logs.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/ts_mon/__init__.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/ts_mon/common/errors.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/ts_mon/common/http_metrics.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/ts_mon/common/interface.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/ts_mon/common/metrics.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/ts_mon/common/monitors.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/ts_mon/common/standard_metrics.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/ts_mon/config.py
[modify] https://crrev.com/f608998325f9c7eca8a7b418b2ea71fab36d07e8/client/third_party/infra_libs/utils.py
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 22 2017 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/40f1bb6d7d6b047f25126f7a6193c1944afca672

commit 40f1bb6d7d6b047f25126f7a6193c1944afca672
Author: David Sansome <dsansome@google.com>
Date: Wed Feb 22 04:28:35 2017
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 22 2017 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/build/master.DEPS/+/1435bd0fc1ae3a2d3dfea01329daa82ec59eb976

commit 1435bd0fc1ae3a2d3dfea01329daa82ec59eb976
Author: David Sansome <dsansome@chromium.org>
Date: Wed Feb 22 06:04:12 2017
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 22 2017 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/build/slave.DEPS/+/7aad60fd42d74423158bb55a831b8604ff061d3e

commit 7aad60fd42d74423158bb55a831b8604ff061d3e
Author: David Sansome <dsansome@chromium.org>
Date: Wed Feb 22 06:07:31 2017
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 22 2017 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/build/internal.DEPS/+/3d36e7705569818e4dc22a10cd3b12e6420bedc4

commit 3d36e7705569818e4dc22a10cd3b12e6420bedc4
Author: David Sansome <dsansome@chromium.org>
Date: Wed Feb 22 06:07:31 2017

Comment 11 by dsansome@chromium.org, Mar 6 2017

Status: Fixed (was: Started)
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 14 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0

commit 2dd95f76fcec9fc96c9412dbe396a7ad6de911d0
Author: dsansome <dsansome@chromium.org>
Date: Tue Mar 14 08:36:58 2017

Roll infra_libs and gae_ts_mon in luci-py, and add field_specs to all metrics

BUG= 684974 
TBR=maruel

Review-Url: https://codereview.chromium.org/2705273003

[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/appengine/gce-backend/metrics.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/appengine/machine_provider/metrics.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/appengine/swarming/swarming_bot/bot_code/bot_main.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/appengine/swarming/ts_mon_metrics.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/appengine/third_party/gae_ts_mon/README.md
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/appengine/third_party/gae_ts_mon/__init__.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/appengine/third_party/gae_ts_mon/config.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/appengine/third_party/gae_ts_mon/shared.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/event_mon/protos/goma_stats_pb2.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/httplib2_utils.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/logs/logs.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/ts_mon/__init__.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/ts_mon/common/errors.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/ts_mon/common/http_metrics.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/ts_mon/common/interface.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/ts_mon/common/metrics.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/ts_mon/common/monitors.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/ts_mon/common/standard_metrics.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/ts_mon/config.py
[modify] https://crrev.com/2dd95f76fcec9fc96c9412dbe396a7ad6de911d0/client/third_party/infra_libs/utils.py

Comment 13 by dsansome@chromium.org, Mar 15 2017 

Issue 640404 has been merged into this issue.
►

Sign in to add a comment