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

Refactor Prometheus metric mappings #9948

Merged
merged 15 commits into from
Jan 25, 2019
54 changes: 7 additions & 47 deletions metricbeat/docs/fields.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -18270,66 +18270,26 @@ Total number of temp block cache written by the query.
[[exported-fields-prometheus]]
== Prometheus fields

Stats collected from Prometheus.
Stats scraped from a Prometheus endpoint.



[float]
== prometheus fields




[float]
== stats fields

Stats about the Prometheus server.



[float]
== notifications fields

Notification stats.



*`prometheus.stats.notifications.queue_length`*::
+
--
type: long

Current queue length.


--

*`prometheus.stats.notifications.dropped`*::
+
--
type: long

Number of dropped queue events.


--

*`prometheus.stats.processes.open_fds`*::
*`prometheus.labels.*`*::
+
--
type: long
type: object

Number of open file descriptors.
Prometheus metric labels


--

*`prometheus.stats.storage.chunks_to_persist`*::
*`prometheus.*`*::
+
--
type: long
type: object

Number of memory chunks that are not yet persisted to disk.
Prometheus metric - release: ga


--
Expand Down
4 changes: 0 additions & 4 deletions metricbeat/docs/modules/prometheus.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,5 @@ The following metricsets are available:

* <<metricbeat-metricset-prometheus-collector,collector>>

* <<metricbeat-metricset-prometheus-stats,stats>>

include::prometheus/collector.asciidoc[]

include::prometheus/stats.asciidoc[]

23 changes: 0 additions & 23 deletions metricbeat/docs/modules/prometheus/stats.asciidoc

This file was deleted.

3 changes: 1 addition & 2 deletions metricbeat/docs/modules_list.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ This file is generated! See scripts/docs_collector.py
|<<metricbeat-metricset-postgresql-database,database>>
|<<metricbeat-metricset-postgresql-statement,statement>> beta[]
|<<metricbeat-module-prometheus,Prometheus>> |image:./images/icon-no.png[No prebuilt dashboards] |
.2+| .2+| |<<metricbeat-metricset-prometheus-collector,collector>>
|<<metricbeat-metricset-prometheus-stats,stats>> beta[]
.1+| .1+| |<<metricbeat-metricset-prometheus-collector,collector>>
|<<metricbeat-module-rabbitmq,RabbitMQ>> beta[] |image:./images/icon-yes.png[Prebuilt dashboards are available] |
.4+| .4+| |<<metricbeat-metricset-rabbitmq-connection,connection>> beta[]
|<<metricbeat-metricset-rabbitmq-exchange,exchange>> beta[]
Expand Down
1 change: 0 additions & 1 deletion metricbeat/include/list.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion metricbeat/module/prometheus/_meta/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
period: 10s
hosts: ["localhost:9090"]
metrics_path: /metrics
#namespace: example
#username: "user"
#password: "secret"

Expand Down
16 changes: 12 additions & 4 deletions metricbeat/module/prometheus/_meta/fields.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
- key: prometheus
title: "Prometheus"
description: >
Stats collected from Prometheus.
Stats scraped from a Prometheus endpoint.
short_config: false
release: ga
settings: ["ssl", "http"]
fields:
- name: prometheus
type: group
# Order is important here, labels will match first, the rest are double
Copy link
Member

Choose a reason for hiding this comment

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

What about placing labels in the base level to avoid possible issues with this? This is also the recommendation in ECS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer to keep it like this for now, main reasons are:

  • It seems there is no way to do alias on labels.*
  • If other modules push labels there, it could lead to mapping issues if they are using dots

I want to make sure this module won't end on mapping issues and we can use these metrics reliably, not against this change once the problem is gone (for instance, if/when index per module is implemented).

WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

LGTM, but I wonder some things (not blocking this PR)

It seems there is no way to do alias on labels.*

Why would we want to do it here?

If other modules push labels there, it could lead to mapping issues if they are using dots

I guess this is no different with any other module, maybe we shouldn't have a field for labels in ECS till we don't have a complete solution for these mapping issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this is no different with any other module, maybe we shouldn't have a field for labels in ECS till we don't have a complete solution for these mapping issues.

That could make sense, I still have some doubts on how ECS will play out in cases like this, I would prefer to play it safe on this module and learn from the process with others 😇

- name: prometheus.labels.*
type: object
object_type: keyword
description: >
fields:
Prometheus metric labels
- name: prometheus.*
type: object
object_type: double
Copy link
Member

Choose a reason for hiding this comment

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

👍

object_type_mapping_type: "*"
description: >
Prometheus metric
35 changes: 10 additions & 25 deletions metricbeat/module/prometheus/collector/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"fmt"

"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/cfgwarn"
p "github.com/elastic/beats/metricbeat/helper/prometheus"
"github.com/elastic/beats/metricbeat/mb"
"github.com/elastic/beats/metricbeat/mb/parse"
Expand Down Expand Up @@ -50,20 +49,9 @@ func init() {
type MetricSet struct {
mb.BaseMetricSet
prometheus p.Prometheus
namespace string
}

func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The prometheus collector metricset is beta")

config := struct {
Namespace string `config:"namespace" validate:"required"`
Copy link
Member

Choose a reason for hiding this comment

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

👍

}{}
err := base.Module().UnpackConfig(&config)
if err != nil {
return nil, err
}

prometheus, err := p.NewPrometheusClient(base)
if err != nil {
return nil, err
Expand All @@ -72,42 +60,39 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
return &MetricSet{
BaseMetricSet: base,
prometheus: prometheus,
namespace: config.Namespace,
}, nil
}

func (m *MetricSet) Fetch() ([]common.MapStr, error) {
func (m *MetricSet) Fetch(reporter mb.ReporterV2) {

Choose a reason for hiding this comment

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

exported method MetricSet.Fetch should have comment or be unexported

families, err := m.prometheus.GetFamilies()

if err != nil {
return nil, fmt.Errorf("Unable to decode response from prometheus endpoint")
reporter.Error(fmt.Errorf("Unable to decode response from prometheus endpoint"))
return
}

eventList := map[string]common.MapStr{}

for _, family := range families {
promEvents := GetPromEventsFromMetricFamily(family)
promEvents := getPromEventsFromMetricFamily(family)

for _, promEvent := range promEvents {
if _, ok := eventList[promEvent.labelHash]; !ok {
eventList[promEvent.labelHash] = common.MapStr{}
labelsHash := promEvent.LabelsHash()
if _, ok := eventList[labelsHash]; !ok {
eventList[labelsHash] = common.MapStr{}

// Add labels
if len(promEvent.labels) > 0 {
eventList[promEvent.labelHash]["label"] = promEvent.labels
eventList[labelsHash]["labels"] = promEvent.labels
}
}

eventList[promEvent.labelHash][promEvent.key] = promEvent.value
eventList[labelsHash].Update(promEvent.data)
}
}

// Converts hash list to slice
events := []common.MapStr{}
for _, e := range eventList {
e[mb.NamespaceKey] = m.namespace
events = append(events, e)
reporter.Event(mb.Event{ModuleFields: e})
}

return events, err
}
84 changes: 47 additions & 37 deletions metricbeat/module/prometheus/collector/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestGetPromEventsFromMetricFamily(t *testing.T) {
}
tests := []struct {
Family *dto.MetricFamily
Event PromEvent
Event []PromEvent
}{
{
Family: &dto.MetricFamily{
Expand All @@ -56,13 +56,13 @@ func TestGetPromEventsFromMetricFamily(t *testing.T) {
},
},
},
Event: PromEvent{
key: "http_request_duration_microseconds",
value: common.MapStr{
"value": int64(10),
Event: []PromEvent{
{
data: common.MapStr{
"http_request_duration_microseconds": float64(10),
},
labels: labels,
},
labelHash: labels.String(),
labels: labels,
},
},
{
Expand All @@ -78,12 +78,13 @@ func TestGetPromEventsFromMetricFamily(t *testing.T) {
},
},
},
Event: PromEvent{
key: "http_request_duration_microseconds",
value: common.MapStr{
"value": float64(10),
Event: []PromEvent{
{
data: common.MapStr{
"http_request_duration_microseconds": float64(10),
},
labels: common.MapStr{},
},
labelHash: "#",
},
},
{
Expand All @@ -106,16 +107,22 @@ func TestGetPromEventsFromMetricFamily(t *testing.T) {
},
},
},
Event: PromEvent{
key: "http_request_duration_microseconds",
value: common.MapStr{
"count": uint64(10),
"sum": float64(10),
"percentile": common.MapStr{
"99": float64(10),
Event: []PromEvent{
{
data: common.MapStr{
"http_request_duration_microseconds_count": uint64(10),
"http_request_duration_microseconds_sum": float64(10),
},
labels: common.MapStr{},
},
{
data: common.MapStr{
"http_request_duration_microseconds": float64(10),
},
labels: common.MapStr{
"quantile": float64(0.99),
},
},
labelHash: "#",
},
},
{
Expand All @@ -138,16 +145,20 @@ func TestGetPromEventsFromMetricFamily(t *testing.T) {
},
},
},
Event: PromEvent{
key: "http_request_duration_microseconds",
value: common.MapStr{
"count": uint64(10),
"sum": float64(10),
"bucket": common.MapStr{
"0.99": uint64(10),
Event: []PromEvent{
{
data: common.MapStr{
"http_request_duration_microseconds_count": uint64(10),
"http_request_duration_microseconds_sum": float64(10),
},
labels: common.MapStr{},
},
{
data: common.MapStr{
"http_request_duration_microseconds": uint64(10),
},
labels: common.MapStr{"le": float64(0.99)},
},
labelHash: "#",
},
},
{
Expand All @@ -169,20 +180,19 @@ func TestGetPromEventsFromMetricFamily(t *testing.T) {
},
},
},
Event: PromEvent{
key: "http_request_duration_microseconds",
value: common.MapStr{
"value": float64(10),
Event: []PromEvent{
{
data: common.MapStr{
"http_request_duration_microseconds": float64(10),
},
labels: labels,
},
labelHash: labels.String(),
labels: labels,
},
},
}

for _, test := range tests {
event := GetPromEventsFromMetricFamily(test.Family)
assert.Equal(t, len(event), 1)
assert.Equal(t, event[0], test.Event)
event := getPromEventsFromMetricFamily(test.Family)
assert.Equal(t, test.Event, event)
}
}
Loading