Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Views API Prototype #596

Merged
merged 47 commits into from
Aug 3, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
7419bda
view
lzchen Apr 16, 2020
aa7562a
remove label keys
lzchen Apr 17, 2020
4a7f6dd
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
lzchen Apr 20, 2020
e43a984
Remove meter ref, aggregator for
lzchen Apr 21, 2020
4d4351f
view manager as part of meter
lzchen Apr 21, 2020
e667bc8
fix tests
lzchen Apr 21, 2020
5d9ae6f
seperate aggregations from aggregators
lzchen Apr 24, 2020
554dafb
default aggregations
lzchen Apr 24, 2020
f8997ca
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
lzchen May 4, 2020
8331f1b
Add label key logic
lzchen May 4, 2020
d93f159
aggregate config and histogram
lzchen May 7, 2020
53cc0f7
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
lzchen May 7, 2020
e966f7e
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
lzchen Jul 1, 2020
adce43e
views
lzchen Jul 2, 2020
864b644
fix observer
lzchen Jul 2, 2020
fea636f
fix lastvalue
lzchen Jul 2, 2020
af6d003
fix tests
lzchen Jul 7, 2020
176aa55
fix tests
lzchen Jul 7, 2020
12402f7
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
lzchen Jul 7, 2020
8e3a903
changelog
lzchen Jul 7, 2020
9e7332f
fix tests
lzchen Jul 7, 2020
3eb7dcd
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
lzchen Jul 7, 2020
ac3734c
lint
lzchen Jul 7, 2020
82a154c
lint
lzchen Jul 7, 2020
55ad347
lint
lzchen Jul 7, 2020
fbeaf34
lint
lzchen Jul 7, 2020
778223e
lint
lzchen Jul 7, 2020
4875769
lint
lzchen Jul 7, 2020
f793247
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
lzchen Jul 7, 2020
d7fbcef
Merge branch 'master' into views
lzchen Jul 8, 2020
45e96f3
fix state
lzchen Jul 10, 2020
0b22ef7
Merge branch 'views' of https://github.com/lzchen/opentelemetry-pytho…
lzchen Jul 10, 2020
2a55926
fix test
lzchen Jul 10, 2020
b74fb2d
Merge branch 'master' into views
lzchen Jul 10, 2020
a510eb0
Create new aggregator per ViewData, config dict
cnnradams Jul 28, 2020
54594c8
ViewData references, equality updates
cnnradams Jul 30, 2020
8baa90e
Histogram updates
cnnradams Jul 30, 2020
765e8ef
updates
cnnradams Jul 31, 2020
bba29d5
fix batcher
cnnradams Jul 31, 2020
d609602
Merge pull request #8 from cnnradams/views
lzchen Jul 31, 2020
4b49cb7
Fix View hash
c24t Aug 1, 2020
de145ce
Wrap, blacken
c24t Aug 1, 2020
93c3808
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
lzchen Aug 3, 2020
2962561
Merge branch 'views' of https://github.com/lzchen/opentelemetry-pytho…
lzchen Aug 3, 2020
58212ba
fix teests
lzchen Aug 3, 2020
f0e74b7
test
lzchen Aug 3, 2020
a8ab8de
lint
lzchen Aug 3, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/examples/basic_meter/basic_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
- How to configure a meter passing a sateful or stateless.
- How to configure an exporter and how to create a controller.
- How to create some metrics intruments and how to capture data with them.
- How to use views to specify aggregation types for each metric instrument.
"""
import sys
import time
Expand Down Expand Up @@ -56,7 +57,7 @@ def usage(argv):
# over the process lifetime. If false, metrics are reset at the beginning of
# each collection interval.
metrics.set_meter_provider(MeterProvider())
meter = metrics.get_meter(__name__, batcher_mode == "stateful")
meter = metrics.get_meter(__name__, stateful)

# Exporter to export metrics to the console
exporter = ConsoleMetricsExporter()
Expand All @@ -72,7 +73,6 @@ def usage(argv):
unit="1",
value_type=int,
metric_type=Counter,
label_keys=("environment",),
)

requests_size = meter.create_metric(
Expand All @@ -81,7 +81,6 @@ def usage(argv):
unit="1",
value_type=int,
metric_type=Measure,
label_keys=("environment",),
)

# Labels are used to identify key-values that are associated with a specific
Expand All @@ -101,4 +100,5 @@ def usage(argv):

requests_counter.add(35, testing_labels)
requests_size.record(2, testing_labels)
time.sleep(5)

input("...\n")
25 changes: 13 additions & 12 deletions docs/examples/basic_meter/calling_conventions.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
import time

from opentelemetry import metrics
from opentelemetry.sdk.metrics import Counter, Measure, MeterProvider
from opentelemetry.sdk.metrics import Counter, MeterProvider
from opentelemetry.sdk.metrics.export.aggregate import CountAggregation
from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter
from opentelemetry.sdk.metrics.export.controller import PushController
from opentelemetry.sdk.metrics.view import View

# Use the meter type provided by the SDK package
metrics.set_meter_provider(MeterProvider())
Expand All @@ -35,16 +37,6 @@
unit="1",
value_type=int,
metric_type=Counter,
label_keys=("environment",),
)

requests_size = meter.create_metric(
name="requests_size",
description="size of requests",
unit="1",
value_type=int,
metric_type=Measure,
label_keys=("environment",),
)

clicks_counter = meter.create_metric(
Expand All @@ -53,11 +45,18 @@
unit="1",
value_type=int,
metric_type=Counter,
label_keys=("environment",),
)

labels = {"environment": "staging"}

# Views are used to define an aggregation type to use for a specific metric
counter_view = View(requests_counter, CountAggregation())
clicks_view = View(clicks_counter, CountAggregation())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the creating a view requires a reference to the metric instrument itself. This means we can create a view only after creating the instrument itself.

Can we instead create Views with the name/description of the instrument? This can also allow us to load Views setting from json/yml etc.
{
"metername" : "MeterForHttpLib"
, "instrumentname", "RequestCounts"
, keys : ["httpurl","httpstatus"]
"aggregation":
{
Type:, "Histogram",
Options:
{
histogramboundaries....
}
}
}
When a meter/instrument matching the above is actually created in the program, it'll get its View config automatically from the config.

Thougts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think that the application owner may not have access to requests_counter object, if it was an instrument created inside another library. Only that library will have access to it.
The application owner will have to use "metername + instrumentname" or similar to specify an instrument.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what I did for .net prototype.

MetricViewRegistry metricViewRegistry = new MetricViewRegistry();
metricViewRegistry.AddMetricView(new MetricView() { MeterName = "library1", InstrumentName = "testCounter", Aggregation = Aggregation.SUM, Keys = new List<string>() { "k1", "k2" } });
metricViewRegistry.AddMetricView(new MetricView() { MeterName = "library1", InstrumentName = "testCounter", Aggregation = Aggregation.SUM, Keys = new List<string>() { "k1" } });
metricViewRegistry.AddMetricView(new MetricView() { MeterName = "library1", InstrumentName = "testCounter", Aggregation = Aggregation.SUM, Keys = new List<string>() { "k2" } });

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a possible improvement in the future. For the prototype, I believe it is okay to have just the basic functionality and iterate after.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cijothomas I like what you posted here. We should be imagining that views will be configured from a .yaml file, which many but not all users will want.


# Register the views to the view manager to use the views
meter.register_view(counter_view)
meter.register_view(clicks_view)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qn on requests_counter. This counter instrument is created much before a view involving it is created and registered. What would happen to the requests_counter.add() calls - how would they be aggregated, before the register_view call occurred?

Should we require that all views must be registered beforehand?

Copy link
Contributor Author

@lzchen lzchen Apr 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we require that all views must be registered beforehand?

Good question. Yes I believe we should enforce this as a requirement.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dynamic view management would certainly be more challenging. Let's leave it out for now.


print("Updating using direct calling convention...")
# You can record metrics directly using the metric instrument. You pass in
# labels that you would like to record for.
Expand All @@ -80,3 +79,5 @@
# specified labels for each.
meter.record_batch(labels, ((requests_counter, 50), (clicks_counter, 70)))
time.sleep(5)

input("...\n")
5 changes: 0 additions & 5 deletions docs/examples/basic_meter/observer.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,8 @@
from opentelemetry import metrics
from opentelemetry.sdk.metrics import MeterProvider
from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter
from opentelemetry.sdk.metrics.export.batcher import UngroupedBatcher
from opentelemetry.sdk.metrics.export.controller import PushController

# Configure a stateful batcher
batcher = UngroupedBatcher(stateful=True)
metrics.set_meter_provider(MeterProvider())
meter = metrics.get_meter(__name__)
exporter = ConsoleMetricsExporter()
Expand All @@ -45,7 +42,6 @@ def get_cpu_usage_callback(observer):
description="per-cpu usage",
unit="1",
value_type=float,
label_keys=("cpu_number",),
)


Expand All @@ -61,7 +57,6 @@ def get_ram_usage_callback(observer):
description="RAM memory usage",
unit="1",
value_type=float,
label_keys=(),
)

input("Metrics will be printed soon. Press a key to finish...\n")
2 changes: 0 additions & 2 deletions opentelemetry-api/src/opentelemetry/metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,6 @@ def create_metric(
unit: str,
value_type: Type[ValueT],
metric_type: Type[MetricT],
label_keys: Sequence[str] = (),
enabled: bool = True,
) -> "Metric":
"""Creates a ``metric_kind`` metric with type ``value_type``.
Expand All @@ -303,7 +302,6 @@ def create_metric(
(https://unitsofmeasure.org/ucum.html).
value_type: The type of values being recorded by the metric.
metric_type: The type of metric being created.
label_keys: The keys for the labels with dynamic values.
enabled: Whether to report the metric by default.
Returns: A new ``metric_type`` metric with values of ``value_type``.
"""
Expand Down
Loading