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

Remove need for autodiscovery to monitor Istiod service. #4924

Merged
merged 2 commits into from
Jan 3, 2023
Merged

Remove need for autodiscovery to monitor Istiod service. #4924

merged 2 commits into from
Jan 3, 2023

Conversation

gsantoro
Copy link
Contributor

@gsantoro gsantoro commented Jan 3, 2023 •
edited
Loading

What does this PR do?

Remove need for autodiscovery to monitor Istiod service.

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.

Author's Checklist

  • [ ]

How to test this PR locally

  1. Setup Istio by following the steps at Istio logs #3632
  2. create a data view for the index pattern metrics-istio.istiod*
  3. Switch to Discover and select the data view created at step 2
  4. Check the field error.message is empty for all metric events.
  5. Add a filter istio.istiod.metrics.pilot_inbound_updates.counter: exists to view a sample of the dataset for one of the metrics used in the [Metrics Istio] Overview dashboard
  6. Check the dashboard [Metrics Istio] Overview has no errors and some actual data.

Related issues

Screenshots

@gsantoro gsantoro added enhancement New feature or request draft Draft Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team labels Jan 3, 2023
@gsantoro gsantoro requested a review from a team as a code owner January 3, 2023 12:08
@gsantoro gsantoro self-assigned this Jan 3, 2023
@elasticmachine
Copy link

elasticmachine commented Jan 3, 2023 •
edited
Loading

💚 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-03T12:10:10.631+0000

  • Duration: 14 min 24 sec

Test stats 🧪

Test Results
Failed 0
Passed 14
Skipped 0
Total 14

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@elasticmachine
Copy link

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 100.0% (3/3) 💚
Classes 100.0% (3/3) 💚
Methods 90.0% (27/30) 👎 -6.667
Lines 97.472% (347/356) 👎 -2.528
Conditionals 100.0% (0/0) 💚

ChrsMark
ChrsMark approved these changes Jan 3, 2023
View reviewed changes
Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

The change looks good to me!

Please make sure that the title of the PR is representative (are we doing it GA?) and that there is a clear path forward. If we want to give some time to the integration until it's officially GA consider filing a new GH issue just as a reminder for this.

nit: consider adding some manual testing notes for this PR, something simple but enough to bootstrap someone that might need to manually test or reproduce this package.

packages/istio/manifest.yml
@@ -3,7 +3,7 @@ name: istio
title: Istio
description: Collect logs and metrics from the service mesh Istio with Elastic Agent.
type: integration
version: 0.2.1
version: 0.2.2
release: beta
Copy link
Member

Choose a reason for hiding this comment

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

I guess since it will be still beta you can change the title of the PR accordingly?

packages/istio/data_stream/istiod_metrics/manifest.yml
@@ -20,7 +20,7 @@ streams:
required: true
show_user: true
default:
- ${kubernetes.pod.ip}:15014
- istiod.istio-system:15014
Copy link
Member

Choose a reason for hiding this comment

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

👍🏼

@gsantoro gsantoro changed the title Make Istio integration GA Remove need for autodiscovery to monitor Istiod service. Jan 3, 2023
@gsantoro gsantoro merged commit 9cded36 into elastic:main Jan 3, 2023
@gsantoro gsantoro deleted the feature/4678_istio_0.2.2 branch January 3, 2023 16:16
@elasticmachine
Copy link

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

@gsantoro gsantoro mentioned this pull request Jan 5, 2023
Closed
14 tasks
@gsantoro gsantoro removed the draft Draft label Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrsMark ChrsMark ChrsMark approved these changes

@tetianakravchenko tetianakravchenko tetianakravchenko approved these changes

Assignees

@gsantoro gsantoro

Labels
enhancement New feature or request Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gsantoro @elasticmachine @ChrsMark @tetianakravchenko