Skip to content

Migrate Prometheus Module to Package #114

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 28 commits into from
Jul 9, 2020
Merged

Migrate Prometheus Module to Package #114

merged 28 commits into from
Jul 9, 2020

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Jun 25, 2020 •
edited
Loading

Implementing Prometheus package migration

Screenshots:
Screenshot 2020-06-29 at 20 20 32
Screenshot 2020-06-29 at 20 22 54
Screenshot 2020-06-29 at 20 22 28
Screenshot 2020-07-06 at 12 37 16
ChrsMark added 7 commits June 25, 2020 10:57
@ChrsMark
Initial commit
9c57bd8 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Fix configs
3c4e63f 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Fix version
e5bcf96 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Add README.md
87168e0 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
fix fields
5776724 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Fix link
a5dcc22 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
fix fields
f829309 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark marked this pull request as draft June 25, 2020 09:15
@elasticmachine
Copy link

elasticmachine commented Jun 25, 2020 •
edited
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Branch indexing]

  • Start Time: 2020-07-09T01:44:54.158+0000

  • Duration: 3 min 51 sec
@ChrsMark
Improve docs
b715336 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@mtojek mtojek added the Team:Integrations Label for the Integrations team label Jun 26, 2020
ChrsMark added 7 commits June 29, 2020 15:50
@ChrsMark
Merge remote-tracking branch 'upstream/master' into migrate_prometheus
cfb349c
@ChrsMark
Fix fields
d6f1045 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Fix typo
e8073d0 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
fix docs
8992d0d 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Add sample json
df846cf 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Add more data JSONs
dac3cf8 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Add ssl enabled setting
f17e6fa 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark marked this pull request as ready for review June 29, 2020 17:24
@elasticmachine
Copy link

Pinging @elastic/integrations (Team:Integrations)
mtojek
mtojek reviewed Jun 30, 2020
View reviewed changes
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

I left a couple of notes.

Could you please post a screenshot with integration's configuration form in Kibana?
packages/prometheus/dataset/remote_write/manifest.yml Outdated
default: "9201"
- name: ssl.enabled
type: bool
title: Ssl Enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Enabled SSL?
packages/prometheus/dataset/remote_write/manifest.yml Outdated
default: false
- name: ssl.certificate
type: text
title: Ssl Certificate
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: SSL certificate
packages/prometheus/dataset/remote_write/manifest.yml Outdated
required: false
show_user: true
default: /etc/pki/server/cert.key
title: Prometheus remote_write metrics
Copy link
Contributor

Choose a reason for hiding this comment

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

Prometheus remote write metrics
packages/prometheus/dataset/remote_write/manifest.yml Outdated
show_user: true
default: /etc/pki/server/cert.key
title: Prometheus remote_write metrics
description: Collect Prometheus remote_write metrics
Copy link
Contributor

Choose a reason for hiding this comment

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

same here
packages/prometheus/docs/README.md Outdated
To scrape metrics from a Prometheus exporter, configure the `hosts` field to it. The path
to retrieve the metrics from (`/metrics` by default) can be configured with `metrics_path`.

```$yml
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this config is not relevant for Elastic Agent anymore. All configs are distributed and generated in Kibana.
packages/prometheus/docs/README.md

When `use_types` and `rate_counters` are enabled, metrics are stored like this:

```$json
Copy link
Contributor

Choose a reason for hiding this comment

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

Yesterday I pushed the tool for generating docs, which supports rendering events from a separate JSON file.

Could you please adjust the README template (import-beats-resources) similarly to #110 ?
packages/prometheus/docs/README.md Outdated

#### Scraping all metrics from a Prometheus server

> ⚠️
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this :D ?
packages/prometheus/docs/README.md Outdated
| dataset.name | Dataset name. | constant_keyword |
| dataset.namespace | Dataset namespace. | constant_keyword |
| dataset.type | Dataset type. | constant_keyword |
| prometheus. | | keyword |
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you miss type somewhere?
packages/prometheus/docs/README.md Outdated
"ecs": {
"version": "1.5.0"
},
"host": {
Copy link
Contributor

Choose a reason for hiding this comment

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

I used to clean all details regarding host. It would make the event smaller.
@mtojek
Copy link
Contributor

mtojek commented Jun 30, 2020

Please also rebase against master.
ChrsMark added 4 commits July 6, 2020 11:02
@ChrsMark
Review changes
88276dd 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Merge remote-tracking branch 'upstream/master' into migrate_prometheus
a9567dd
@ChrsMark
Add sample_event.json files and run mage GenerateDocs
79a84bf 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Tune to breaking changes
1b20cff 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

ChrsMark commented Jul 6, 2020

Thanks for the review @mtojek !

PR has been updated:

  • Added screenshot
  • Addressed comments and rebased
  • Renamed datasources to config_templates
  • Renamed metrics category to datastores
ChrsMark added 2 commits July 6, 2020 13:26
@ChrsMark
Update expected json
73ce668 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
fix minor typo
59cd5b7 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark mentioned this pull request Jul 7, 2020
Merged
ChrsMark added 5 commits July 7, 2020 15:02
@ChrsMark
Make rates settings required
b71aede 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Merge remote-tracking branch 'upstream/master' into migrate_prometheus
4f5bead
@ChrsMark
Add package owners
316ddbe 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Add category
6122c29 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Fix kibana version
98c5a31 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark requested a review from a team July 8, 2020 07:58
@ChrsMark
Fix kibana version
6dca449 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
exekias
exekias reviewed Jul 8, 2020
View reviewed changes
packages/prometheus/dataset/collector/fields/fields.yml
@@ -0,0 +1,11 @@
- name: prometheus
Copy link

Choose a reason for hiding this comment

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

This file seems to be missing some fields. I would expect it to be a mix between x-pack and oss fields.yml's, as we are shipping the basic agents with the agent
Copy link
Member Author

Choose a reason for hiding this comment

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

🤦 good catch, thanks!
exekias
exekias suggested changes Jul 8, 2020
View reviewed changes
Copy link

@exekias exekias left a comment

Choose a reason for hiding this comment

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

fields.yml is incomplete, everything else LGTM!
@ChrsMark
Add missing x-pack fields
e3746df 
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

ChrsMark commented Jul 9, 2020

@exekias I added manually the missing fields and tested manually again to verify nothing is broken. Feel free to have another look :)!
@ChrsMark ChrsMark merged commit 4d00202 into elastic:master Jul 9, 2020
@ChrsMark ChrsMark mentioned this pull request Jul 9, 2020
Merged
@masci masci mentioned this pull request Nov 19, 2021
Closed
8 tasks
@andrewkroh andrewkroh added Integration:prometheus Prometheus New Integration Issue or pull request for creating a new integration package. labels Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Integration:prometheus Prometheus New Integration Issue or pull request for creating a new integration package. Team:Integrations Label for the Integrations team

5 participants

@ChrsMark @elasticmachine @mtojek @exekias @andrewkroh