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.

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

[receiver/elasticsearch] Implement scraping logic #7174

Merged
merged 16 commits into from
Jan 17, 2022
Merged

[receiver/elasticsearch] Implement scraping logic #7174

merged 16 commits into from
Jan 17, 2022

Conversation

BinaryFissionGames
Copy link
Contributor

@BinaryFissionGames BinaryFissionGames commented Jan 14, 2022 •
edited
Loading

Description:

  • Implemented scraping logic.
  • Removed some unavailable JVM metrics (Elasticsearch does not expose all the metrics that the JMX receiver collects).
  • Updated config with some new options.
    • Add support for specifying nodes to scrape.
    • Add support for skipping scraping cluster-level metrics.
    • Add support for generated MetricSettings, allowing individual metrics to be enabled/disabled.

Testing:

  • Unit tests for scraping logic (using golden metrics serialized to disk).
  • Updated advanced config test to include new config options.
  • Manually tested to ensure metrics were being received against Elasticsearch 7.15.2

Documentation:

  • Updated documentation with new config options.

@BinaryFissionGames BinaryFissionGames marked this pull request as ready for review January 14, 2022 13:58
djaglowski
djaglowski reviewed Jan 14, 2022
View reviewed changes
receiver/elasticsearchreceiver/scraper.go Show resolved Hide resolved
receiver/elasticsearchreceiver/go.mod Outdated Show resolved Hide resolved
receiver/elasticsearchreceiver/scraper.go Outdated Show resolved Hide resolved
receiver/elasticsearchreceiver/scraper.go

r.metricsBuilder.RecordJvmThreadsCountDataPoint(r.now, info.JVMInfo.JVMThreadInfo.Count)

r.metricsBuilder.EmitNodeMetrics(ilms.Metrics())
Copy link
Member

Choose a reason for hiding this comment

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

Great to see so much done in such few lines. Thanks @dmitryax!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seriously, the new MetricBuilder stuff is really well done!

receiver/elasticsearchreceiver/scraper_test.go Outdated Show resolved Hide resolved
receiver/elasticsearchreceiver/scraper_test.go Outdated Show resolved Hide resolved
@BinaryFissionGames BinaryFissionGames force-pushed the elasticsearchreceiver-scraping-logic branch from 885a0a5 to 3a535c4 Compare January 15, 2022 03:51
djaglowski
djaglowski reviewed Jan 17, 2022
View reviewed changes
Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

mdatagen was bumped recently, so you'll need to rebase once more, but this looks good to me.

@BinaryFissionGames BinaryFissionGames force-pushed the elasticsearchreceiver-scraping-logic branch from 16ca922 to 1e1307a Compare January 17, 2022 15:29
@BinaryFissionGames
Copy link
Contributor Author

@djaglowski Rebased; The mysql integration test failures are unrelated, not sure what's going on there.

BinaryFissionGames and others added 16 commits January 17, 2022 13:05
@BinaryFissionGames
Add new config options for controlling included metrics
20bda5b
@BinaryFissionGames
implement scraper
9f982a4
@BinaryFissionGames
use ScrapeErrors to track partial errors
4e0b0c3
@BinaryFissionGames
Return scraper from factory
f39f565
@BinaryFissionGames
lint fixes; test disabling cluster/node metrics
cb5f2a9
@BinaryFissionGames
Tweak test payloads to avoid 0 values
d92ded9 
This helps verify that we are actually geting metric values,
and not just zero values that are unfilled from the struct.
@BinaryFissionGames
add replace directive for scrapertest package; go mod tidy
67a9de5
@BinaryFissionGames
Fix typo in config go doc
c540dac
@BinaryFissionGames
changelog
56857ae
@BinaryFissionGames
fix instrumentation library name prefix
8084fa7
@BinaryFissionGames
move replace directive to bottom of go.mod
faf91bb
@BinaryFissionGames
fix small typo in scraper test error
8e0fdbf
@BinaryFissionGames
rewrite golden
35baa00
@BinaryFissionGames
Remove TestWriteGolden
33b0efc
@BinaryFissionGames
Fix "Cluster health status is invalid" error check
c7b719a
@BinaryFissionGames
run make porto
07b7292
@BinaryFissionGames BinaryFissionGames force-pushed the elasticsearchreceiver-scraping-logic branch from 37ba18d to 07b7292 Compare January 17, 2022 18:05
@djaglowski
Copy link
Member

@BinaryFissionGames Will you rebase once more? The mysql integration test has been temporarily disabled due to instability.

@djaglowski djaglowski added the ready to merge Code review completed; ready to merge by maintainers label Jan 17, 2022
@jpkrohling jpkrohling merged commit 72c3535 into open-telemetry:main Jan 17, 2022
animetauren pushed a commit to animetauren/opentelemetry-collector-contrib that referenced this pull request Apr 4, 2023
split receiver into its own module (open-telemetry#7195)
4071a47 
Adds the receiver component as its own module.

Fixes open-telemetry#7174

---------

Signed-off-by: Alex Boten <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djaglowski djaglowski djaglowski approved these changes

Assignees

@Aneurysm9 Aneurysm9

Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@BinaryFissionGames @djaglowski @jpkrohling @Aneurysm9