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

[Golang] Add Integration Package with expvar Data Stream #4933

Merged
merged 3 commits into from
Feb 1, 2023

Conversation

niraj-crest
Copy link
Contributor

@niraj-crest niraj-crest commented Jan 4, 2023

  • Enhancement

What does this PR do?

  • Added 1 data stream (expvar metrics).
  • Added data collection logic for the data streams.
  • Added the ingest pipeline for the data streams.
  • Mapped fields according to the ECS schema and added Fields metadata in the appropriate yaml files.
  • Added dashboards and visualizations.
  • Added system test cases for the data stream.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

How to test this PR locally

  • Clone integrations repo.
  • Install elastic-package locally.
  • Start elastic stack using elastic-package.
  • Move to integrations/packages/golang) directory.
  • Run the following command to run tests. elastic-package test

Related issues

Screenshots

golang-expvar-dashboard

@elasticmachine
Copy link

elasticmachine commented Jan 4, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-01-05T05:12:45.863+0000

  • Duration: 14 min 37 sec

Test stats 🧪

Test Results
Failed 0
Passed 6
Skipped 0
Total 6

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@kush-elastic
Copy link
Collaborator

/test

Copy link
Collaborator

@kush-elastic kush-elastic left a comment

Choose a reason for hiding this comment

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

LGTM!

@elasticmachine
Copy link

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 100.0% (1/1) 💚
Classes 100.0% (1/1) 💚
Methods 100.0% (8/8) 💚
Lines 95.745% (90/94) 👍 0.181
Conditionals 100.0% (0/0) 💚

- name: golang
type: group
fields:
- name: expvar
Copy link
Contributor

Choose a reason for hiding this comment

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

There are many more fields in the beats sample module https://www.elastic.co/guide/en/beats/metricbeat/current/metricbeat-metricset-golang-expvar.html

Are we dropping out some fields for expvar ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not actually, We looked into those additional fields but those fields aren’t default golang metric fields, for example, beat.cpu.system.ticks is a field of system system.cpu.system.ticks, we ran go module and its not collecting these fields, go has the functionality to expose custom metrics and these could be custom metrics for which we are providing support. If the user has these or any other custom fields exposed in their application, these will be automatically inserted in our document.

@@ -0,0 +1,40 @@
title: Golang expvar metrics
type: logs
Copy link
Collaborator

@lalit-satapathy lalit-satapathy Jan 18, 2023

Choose a reason for hiding this comment

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

Wondering if there a simpler way to re-direct these metrics from filebeat httpjson to metrics-* data-stream or metrics-* data view. Explore, if it is feasible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We tried changing it to “metrics” but we receive the following error. Apparently, it doesn’t work if there is a mismatch between input type and index type, please let us know if there is any other way that we can try.
error

Copy link
Contributor

Choose a reason for hiding this comment

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

elastic/elastic-agent#2208: This is the agent issue opened to track the same

{{/if}}
{{/if}}
processors:
- add_fields:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this processor added here and not as part of ingest pipeline ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need service.address to differentiate data among different hosts if the user has added multiple integrations. We can not add this processor in pipeline since we require accessing the input parameter(hostname) here which can not be done from pipeline.

@ishleenk17 ishleenk17 requested a review from a team January 19, 2023 06:42
@ishleenk17
Copy link
Contributor

ishleenk17 commented Jan 31, 2023

As we have planned to skip some of the gc_pause fields for now.
Please open an issue for the same. So that we have it tracked somewhere, which fields we are skipping and for what reason. @niraj-crest

Copy link
Contributor

@ishleenk17 ishleenk17 left a comment

Choose a reason for hiding this comment

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

Looks good!
Please take care of opening an issue as shared in this comment.

@niraj-crest
Copy link
Contributor Author

niraj-crest commented Jan 31, 2023

As we have planned to skip some of the gc_pause fields for now. Please open an issue for the same. So that we have it tracked somewhere, which fields we are skipping and for what reason. @niraj-crest

As part of this datastream, We don't need to drop any fields. There are some fields in Heap datastream that will be dropped. We'll address them as part of heap PR.

@kush-elastic kush-elastic merged commit ad13051 into elastic:main Feb 1, 2023
@elasticmachine
Copy link

Package golang - 0.2.0 containing this change is available at https://epr.elastic.co/search?package=golang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Integration:golang Golang New Integration Team:Service-Integrations Label for the Service Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants