Skip to content

[Enhancement] [Cisco Duo] Make enhancement in connector with best practices implementation - #4557

Merged
P1llus merged 5 commits into
elastic:mainfrom
vinit-chauhan:package-cisco-duo-1.5.3
Nov 18, 2022
Merged

[Enhancement] [Cisco Duo] Make enhancement in connector with best practices implementation#4557
P1llus merged 5 commits into
elastic:mainfrom
vinit-chauhan:package-cisco-duo-1.5.3

Conversation

@vinit-chauhan

@vinit-chauhan vinit-chauhan commented Nov 4, 2022

Copy link
Copy Markdown
Contributor

Type of change

  • Enhancement

What does this PR do?

Make enhancement in Cisco Duo connector with listed best practices.

Sr. NO. Best Practices
1 Verify best practices test cases are executed
2 Verify all automated tests
3 Verify the cursor value
4 Verify that the data is ingested in kibana for in interval
5 Verify all fields are correctly mapped for all Data streams
6 Verify related.ip field in datastream
7 Verify on-failure on convert processor
8 Verify on-failure on date processor
9 Verify format of title of dashboard
10 Verify Datastream Dashboard
11 Verify filters in visualizations
12 Verify visualizations in different browser
13 Verify README file

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/cisco_duo directory.
  • Run the following command to run tests.

elastic-package test

Related issues

Screenshots

image
image
image
image
@vinit-chauhan
vinit-chauhan requested a review from a team as a code owner November 4, 2022 10:25
@elasticmachine

elasticmachine commented Nov 4, 2022

Copy link
Copy Markdown

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)
@elasticmachine

elasticmachine commented Nov 4, 2022 •
edited
Loading

Copy link
Copy Markdown

💚 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: 2022-11-11T07:47:28.233+0000

  • Duration: 21 min 45 sec

Test stats 🧪

Test Results
Failed 0
Passed 34
Skipped 0
Total 34

🤖 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

elasticmachine commented Nov 4, 2022 •
edited
Loading

Copy link
Copy Markdown

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (5/5) 💚
Files 100.0% (5/5) 💚 2.53
Classes 100.0% (5/5) 💚 2.53
Methods 98.182% (54/55) 👍 6.853
Lines 93.579% (685/732) 👍 1.883
Conditionals 100.0% (0/0) 💚
@elasticmachine

elasticmachine commented Nov 4, 2022 •
edited
Loading

Copy link
Copy Markdown

🚀 Benchmarks report

Package cisco_duo 👍(0) 💚(1) 💔(4)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
admin 7194.24 3759.4 -3434.84 (-47.74%) 💔
auth 2145.92 1414.43 -731.49 (-34.09%) 💔
offline_enrollment 28571.43 18867.92 -9703.51 (-33.96%) 💔
summary 40000 31250 -8750 (-21.88%) 💔

To see the full report comment with /test benchmark fullreport
P1llus
P1llus reviewed Nov 8, 2022
View reviewed changes

@P1llus P1llus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The only issue I see here is the large amount of ignore_failures which has to be removed, as we do not really use them unless necessary in our ingest pipelines.
Comment thread packages/cisco_duo/changelog.yml Outdated
@@ -1,115 +1,120 @@
# newer versions go on top
- version: "1.6.0"
- version: '1.6.1'

@P1llus P1llus Nov 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Because of the dashboard changes etc, feel free to bump this to 1.7.0
Comment thread packages/cisco_duo/changelog.yml Outdated
type: enhancement
link: https://github.com/elastic/integrations/pull/4285
- version: "1.5.2"
- version: '1.5.2'

@P1llus P1llus Nov 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Feel free to change these to double quotes, just so it matches all other integrations
Comment thread packages/cisco_duo/data_stream/admin/elasticsearch/ingest_pipeline/default.yml
copy_from: json.description
if: ctx?.json?.description != null
if: ctx.json?.description != null
ignore_failure: true

@P1llus P1llus Nov 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is a lot of ignore_failures here, which we don't really use in our ingest pipelines unless absolutely necessary, is there any specific reason so many was added?
Comment thread packages/cisco_duo/manifest.yml Outdated
name: cisco_duo
title: Cisco Duo
version: "1.6.0"
version: '1.6.1'

@P1llus P1llus Nov 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Double quotes
@P1llus
P1llus merged commit d07c798 into elastic:main Nov 18, 2022
@vinit-chauhan
vinit-chauhan deleted the package-cisco-duo-1.5.3 branch April 1, 2026 23:57
orestisfl pushed a commit to orestisfl/integrations that referenced this pull request May 15, 2026
@vinit-chauhan
[Enhancement] [Cisco Duo] Make enhancement in connector with best pra…
44735aa 
…ctices implementation (elastic#4557)

* Remove saved library visualizations and add an on_failure processor to the date and convert processors

* Update the Changelog entry

* Run system test

* change as per the review comments
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:cisco_duo Cisco Duo

3 participants

@vinit-chauhan @elasticmachine @P1llus