From a18406e95c05c1b7a1bf096dd51cfd69e629d640 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Tue, 9 Apr 2019 17:04:44 -0700 Subject: [PATCH 1/5] Use default auth for StackdriverStatsExporter (#610) --- .../stackdriver/stats_exporter/__init__.py | 14 +++++++--- .../tests/test_stackdriver_stats.py | 27 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/contrib/opencensus-ext-stackdriver/opencensus/ext/stackdriver/stats_exporter/__init__.py b/contrib/opencensus-ext-stackdriver/opencensus/ext/stackdriver/stats_exporter/__init__.py index bf692095f..39f911eb5 100644 --- a/contrib/opencensus-ext-stackdriver/opencensus/ext/stackdriver/stats_exporter/__init__.py +++ b/contrib/opencensus-ext-stackdriver/opencensus/ext/stackdriver/stats_exporter/__init__.py @@ -22,6 +22,7 @@ from google.api_core.gapic_v1 import client_info from google.cloud import monitoring_v3 +import google.auth from opencensus.common import utils from opencensus.common.monitored_resource import monitored_resource @@ -365,12 +366,16 @@ def get_user_agent_slug(): return "opencensus-python/{}".format(__version__) -def new_stats_exporter(options, interval=None): +def new_stats_exporter(options=None, interval=None): """Get a stats exporter and running transport thread. Create a new `StackdriverStatsExporter` with the given options and start periodically exporting stats to stackdriver in the background. + Fall back to default auth if `options` is null. This will raise + `google.auth.exceptions.DefaultCredentialsError` if default credentials + aren't configured. + See `opencensus.metrics.transport.get_exporter_thread` for details on the transport thread. @@ -380,9 +385,12 @@ def new_stats_exporter(options, interval=None): :type interval: int or float :param interval: Seconds between export calls. - :rtype: :class:`StackdriverStatsExporter` and :class:`PeriodicTask` - :return: A tuple of the exporter and transport thread. + :rtype: :class:`StackdriverStatsExporter` + :return: The newly-created exporter. """ + if options is None: + _, project_id = google.auth.default() + options = Options(project_id=project_id) if str(options.project_id).strip() == "": raise ValueError(ERROR_BLANK_PROJECT_ID) diff --git a/contrib/opencensus-ext-stackdriver/tests/test_stackdriver_stats.py b/contrib/opencensus-ext-stackdriver/tests/test_stackdriver_stats.py index 8fbaeb95f..9f8cb6c8c 100644 --- a/contrib/opencensus-ext-stackdriver/tests/test_stackdriver_stats.py +++ b/contrib/opencensus-ext-stackdriver/tests/test_stackdriver_stats.py @@ -17,6 +17,7 @@ import unittest from google.cloud import monitoring_v3 +import google.auth from opencensus.common import utils from opencensus.common.version import __version__ @@ -115,6 +116,32 @@ def test_constructor_param(self): default_monitoring_labels=default_labels)) self.assertEqual(exporter.options.project_id, project_id) + def test_null_options(self): + # Check that we don't suppress auth errors + auth_error = google.auth.exceptions.DefaultCredentialsError + mock_auth_error = mock.Mock() + mock_auth_error.side_effect = auth_error + with mock.patch('opencensus.ext.stackdriver.stats_exporter' + '.google.auth.default', mock_auth_error): + with self.assertRaises(auth_error): + stackdriver.new_stats_exporter() + + # Check that we get the default credentials' project ID + mock_auth_ok = mock.Mock() + mock_auth_ok.return_value = (None, 123) + with mock.patch('opencensus.ext.stackdriver.stats_exporter' + '.google.auth.default', mock_auth_ok): + sdse = stackdriver.new_stats_exporter() + self.assertEqual(sdse.options.project_id, 123) + + # Check that we raise if auth works but the project is empty + mock_auth_no_project = mock.Mock() + mock_auth_no_project.return_value = (None, '') + with mock.patch('opencensus.ext.stackdriver.stats_exporter' + '.google.auth.default', mock_auth_no_project): + with self.assertRaises(ValueError): + stackdriver.new_stats_exporter() + def test_blank_project(self): self.assertRaises(ValueError, stackdriver.new_stats_exporter, stackdriver.Options(project_id="")) From 969417dfcf33461e555b016b9040fa26ef58edaf Mon Sep 17 00:00:00 2001 From: Sam Neubardt Date: Tue, 9 Apr 2019 21:05:05 -0400 Subject: [PATCH 2/5] Allow empty metric descriptor label keys (#611) --- opencensus/metrics/export/metric_descriptor.py | 4 ++-- tests/unit/metrics/export/test_metric_descriptor.py | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/opencensus/metrics/export/metric_descriptor.py b/opencensus/metrics/export/metric_descriptor.py index cbb258547..5bde7bb23 100644 --- a/opencensus/metrics/export/metric_descriptor.py +++ b/opencensus/metrics/export/metric_descriptor.py @@ -130,8 +130,8 @@ def __init__(self, name, description, unit, type_, label_keys): if type_ not in MetricDescriptorType: raise ValueError("Invalid type") - if not label_keys: - raise ValueError("label_keys must not be empty or null") + if label_keys is None: + raise ValueError("label_keys must not be None") if any(key is None for key in label_keys): raise ValueError("label_keys must not contain null keys") diff --git a/tests/unit/metrics/export/test_metric_descriptor.py b/tests/unit/metrics/export/test_metric_descriptor.py index 30eef9e2b..2a5f7b80c 100644 --- a/tests/unit/metrics/export/test_metric_descriptor.py +++ b/tests/unit/metrics/export/test_metric_descriptor.py @@ -53,6 +53,11 @@ def test_null_label_keys(self): NAME, DESCRIPTION, UNIT, metric_descriptor.MetricDescriptorType.GAUGE_DOUBLE, None) + def test_empty_label_keys(self): + metric_descriptor.MetricDescriptor( + NAME, DESCRIPTION, UNIT, + metric_descriptor.MetricDescriptorType.GAUGE_DOUBLE, []) + def test_null_label_key_values(self): with self.assertRaises(ValueError): metric_descriptor.MetricDescriptor( From c90b4b6c17e26bd27bb5ba284d53e3e09e46536d Mon Sep 17 00:00:00 2001 From: Armin Berres <20811121+aberres@users.noreply.github.com> Date: Thu, 11 Apr 2019 17:05:25 +0200 Subject: [PATCH 3/5] Allow to pass x-b3-sampled as string (#595) While x-b3-sampled should be '0' or '1' some implementations pass a string like 'True' or 'true'. --- opencensus/trace/propagation/b3_format.py | 8 +-- .../unit/trace/propagation/test_b3_format.py | 49 +++++++++++++------ 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/opencensus/trace/propagation/b3_format.py b/opencensus/trace/propagation/b3_format.py index d2369d52b..cd175efa5 100644 --- a/opencensus/trace/propagation/b3_format.py +++ b/opencensus/trace/propagation/b3_format.py @@ -62,10 +62,10 @@ def from_headers(self, headers): sampled = headers.get(_SAMPLED_KEY) if sampled is not None: - if len(sampled) != 1: - return SpanContext(from_header=False) - - sampled = sampled in ('1', 'd') + # The specification encodes an enabled tracing decision as "1". + # In the wild pre-standard implementations might still send "true". + # "d" is set in the single header case when debugging is enabled. + sampled = sampled.lower() in ('1', 'd', 'true') else: # If there's no incoming sampling decision, it was deferred to us. # Even though we set it to False here, we might still sample diff --git a/tests/unit/trace/propagation/test_b3_format.py b/tests/unit/trace/propagation/test_b3_format.py index 68a38f412..75ce604ed 100644 --- a/tests/unit/trace/propagation/test_b3_format.py +++ b/tests/unit/trace/propagation/test_b3_format.py @@ -30,23 +30,44 @@ def test_from_headers_no_headers(self): def test_from_headers_keys_exist(self): test_trace_id = '6e0c63257de34c92bf9efcd03927272e' test_span_id = '00f067aa0ba902b7' - test_sampled = '1' - headers = { - b3_format._TRACE_ID_KEY: test_trace_id, - b3_format._SPAN_ID_KEY: test_span_id, - b3_format._SAMPLED_KEY: test_sampled, - } + for test_sampled in ['1', 'True', 'true', 'd']: + headers = { + b3_format._TRACE_ID_KEY: test_trace_id, + b3_format._SPAN_ID_KEY: test_span_id, + b3_format._SAMPLED_KEY: test_sampled, + } - propagator = b3_format.B3FormatPropagator() - span_context = propagator.from_headers(headers) + propagator = b3_format.B3FormatPropagator() + span_context = propagator.from_headers(headers) - self.assertEqual(span_context.trace_id, test_trace_id) - self.assertEqual(span_context.span_id, test_span_id) - self.assertEqual( - span_context.trace_options.enabled, - bool(test_sampled) - ) + self.assertEqual(span_context.trace_id, test_trace_id) + self.assertEqual(span_context.span_id, test_span_id) + self.assertEqual( + span_context.trace_options.enabled, + True + ) + + def test_from_headers_keys_exist_disabled_sampling(self): + test_trace_id = '6e0c63257de34c92bf9efcd03927272e' + test_span_id = '00f067aa0ba902b7' + + for test_sampled in ['0', 'False', 'false', None]: + headers = { + b3_format._TRACE_ID_KEY: test_trace_id, + b3_format._SPAN_ID_KEY: test_span_id, + b3_format._SAMPLED_KEY: test_sampled, + } + + propagator = b3_format.B3FormatPropagator() + span_context = propagator.from_headers(headers) + + self.assertEqual(span_context.trace_id, test_trace_id) + self.assertEqual(span_context.span_id, test_span_id) + self.assertEqual( + span_context.trace_options.enabled, + False + ) def test_from_headers_keys_not_exist(self): propagator = b3_format.B3FormatPropagator() From 0fcae52646f58cc6331167ee1619f68155da20b5 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 11 Apr 2019 16:33:33 -0700 Subject: [PATCH 4/5] Allow TimeSeries to have empty label values (#614) --- opencensus/metrics/export/time_series.py | 4 +-- tests/unit/metrics/export/test_time_series.py | 2 -- tests/unit/stats/test_metric_utils.py | 33 +++++++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/opencensus/metrics/export/time_series.py b/opencensus/metrics/export/time_series.py index 860d885cf..022f83197 100644 --- a/opencensus/metrics/export/time_series.py +++ b/opencensus/metrics/export/time_series.py @@ -38,8 +38,8 @@ class TimeSeries(object): """ # noqa def __init__(self, label_values, points, start_timestamp): - if not label_values: - raise ValueError("label_values must not be null or empty") + if label_values is None: + raise ValueError("label_values must not be None") if not points: raise ValueError("points must not be null or empty") self._label_values = label_values diff --git a/tests/unit/metrics/export/test_time_series.py b/tests/unit/metrics/export/test_time_series.py index 52646b3e9..b3948361a 100644 --- a/tests/unit/metrics/export/test_time_series.py +++ b/tests/unit/metrics/export/test_time_series.py @@ -49,8 +49,6 @@ def test_init_invalid(self): time_series.TimeSeries(LABEL_VALUES, POINTS, None) with self.assertRaises(ValueError): time_series.TimeSeries(None, POINTS, START_TIMESTAMP) - with self.assertRaises(ValueError): - time_series.TimeSeries([], POINTS, START_TIMESTAMP) with self.assertRaises(ValueError): time_series.TimeSeries(LABEL_VALUES, None, START_TIMESTAMP) with self.assertRaises(ValueError): diff --git a/tests/unit/stats/test_metric_utils.py b/tests/unit/stats/test_metric_utils.py index 9c81eb0ed..71cd6bbf4 100644 --- a/tests/unit/stats/test_metric_utils.py +++ b/tests/unit/stats/test_metric_utils.py @@ -171,3 +171,36 @@ def test_view_data_to_metric(self): ] for args in args_list: self.do_test_view_data_to_metric(*args) + + def test_convert_view_without_labels(self): + mock_measure = mock.Mock(spec=measure.MeasureFloat) + mock_aggregation = mock.Mock(spec=aggregation.DistributionAggregation) + mock_aggregation.aggregation_type = aggregation.Type.DISTRIBUTION + + vd = mock.Mock(spec=view_data.ViewData) + vd.view = view.View( + name=mock.Mock(), + description=mock.Mock(), + columns=[], + measure=mock_measure, + aggregation=mock_aggregation) + vd.start_time = '2019-04-11T22:33:44.555555Z' + + mock_point = mock.Mock(spec=point.Point) + mock_point.value = mock.Mock(spec=value.ValueDistribution) + + mock_agg = mock.Mock(spec=aggregation_data.SumAggregationDataFloat) + mock_agg.to_point.return_value = mock_point + + vd.tag_value_aggregation_data_map = {tuple(): mock_agg} + + current_time = '2019-04-11T22:33:55.666666Z' + metric = metric_utils.view_data_to_metric(vd, current_time) + + self.assertEqual(metric.descriptor.label_keys, []) + self.assertEqual(len(metric.time_series), 1) + [ts] = metric.time_series + self.assertEqual(ts.label_values, []) + self.assertEqual(len(ts.points), 1) + [pt] = ts.points + self.assertEqual(pt, mock_point) From cd0eb49e3871d3443d75df6d34e2be668cc5adae Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 11 Apr 2019 16:48:09 -0700 Subject: [PATCH 5/5] Update versions, changelogs for 0.4.1 and stackdriver exporter 0.2.1. --- CHANGELOG.md | 7 +++++++ contrib/opencensus-ext-stackdriver/CHANGELOG.md | 5 +++++ contrib/opencensus-ext-stackdriver/version.py | 2 +- opencensus/common/version/__init__.py | 2 +- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c879b0928..7bb4a0ee3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +## 0.4.0 +Released 2019-04-11 + +- Allow for metrics with empty label keys and values + ([#611](https://github.com/census-instrumentation/opencensus-python/pull/611)) + ([#614](https://github.com/census-instrumentation/opencensus-python/pull/614)) + ## 0.4.0 Released 2019-04-08 diff --git a/contrib/opencensus-ext-stackdriver/CHANGELOG.md b/contrib/opencensus-ext-stackdriver/CHANGELOG.md index 7a3c201d3..09252649d 100644 --- a/contrib/opencensus-ext-stackdriver/CHANGELOG.md +++ b/contrib/opencensus-ext-stackdriver/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +## 0.2.0 +Released 2019-04-11 +- Don't require exporter options, fall back to default GCP auth + ([#610](https://github.com/census-instrumentation/opencensus-python/pull/610)) + ## 0.2.0 Released 2019-04-08 diff --git a/contrib/opencensus-ext-stackdriver/version.py b/contrib/opencensus-ext-stackdriver/version.py index 63922d675..20fd9b8ed 100644 --- a/contrib/opencensus-ext-stackdriver/version.py +++ b/contrib/opencensus-ext-stackdriver/version.py @@ -12,4 +12,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -__version__ = '0.2.0' +__version__ = '0.2.1' diff --git a/opencensus/common/version/__init__.py b/opencensus/common/version/__init__.py index d79b9cb9c..8048498cd 100644 --- a/opencensus/common/version/__init__.py +++ b/opencensus/common/version/__init__.py @@ -12,4 +12,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -__version__ = '0.4.0' +__version__ = '0.4.1'