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

DDSaaS: Adyen: Integration v1.0.0 #18674

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

DDSaaS: Adyen: Integration v1.0.0 #18674

wants to merge 12 commits into from

Conversation

avaid-crest
Copy link
Contributor

@avaid-crest avaid-crest commented Sep 26, 2024 •
edited
Loading

What does this PR do?

This is a initial release PR of Adyen integration including all the required assets i.e. Pipeline, Dashboards and Monitors

Additional Notes

  • This integration is webhook based so Crawler will not be used.
  • The Datadog team will implement the appropriate component for generating URLs. A URL will be created having the ddsource query parameter set to adyen.
  • The assets are available in our sandbox and can be shared separately with the required teams.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Changelog entries must be created for modifications to shipped code
  • Add the qa/skip-qa label if the PR doesn't need to be tested during QA.
  • If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

@avaid-crest avaid-crest marked this pull request as ready for review September 26, 2024 13:44
@avaid-crest avaid-crest requested review from a team as code owners September 26, 2024 13:44
@drichards-87 drichards-87 added the editorial review Waiting on a more in-depth review from a docs team editor label Sep 26, 2024
@drichards-87
Copy link
Contributor

Created a Jira card for Docs Team editorial review.

brunorenier
brunorenier reviewed Sep 30, 2024
View reviewed changes
Copy link

@brunorenier brunorenier left a comment

Choose a reason for hiding this comment

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

I have a couple comments all related to the fact that notificationItems is an array.
Are we guaranteed there will always be a single entry?

adyen/assets/logs/adyen.yaml Show resolved Hide resolved
adyen/assets/logs/adyen.yaml Show resolved Hide resolved
adyen/assets/logs/adyen_tests.yaml Show resolved Hide resolved
@brunorenier brunorenier added the assets/deploy-logs-staging ONLY USED BY Logs Backend - Validates that a PR is OK to go to staging label Oct 1, 2024
brunorenier
brunorenier previously approved these changes Oct 2, 2024
View reviewed changes
Copy link

@brunorenier brunorenier left a comment

Choose a reason for hiding this comment

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

LGTM for the logs part.

gunterd
gunterd reviewed Nov 18, 2024
View reviewed changes
Copy link
Contributor

@gunterd gunterd 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 to me, pending docs approval

gunterd
gunterd previously approved these changes Nov 18, 2024
View reviewed changes
gunterd
gunterd previously approved these changes Dec 2, 2024
View reviewed changes
estherk15
estherk15 reviewed Dec 2, 2024
View reviewed changes
Copy link
Contributor

@estherk15 estherk15 left a comment

Choose a reason for hiding this comment

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

The monitor descriptions need more context, it should follow best practices:

Provides extra context around the failure mode and also about the impact this mode can have on the system. It should allow users to understand at a glance whether it is relevant or not for them to create a monitor out of it.

  • This is not a copy of the title.
  • Define the problem stated by the title.
  • Answer why this is an issue worth alerting on.
  • Describe the impact of the problem.

Most of the messages for these monitors fit the requirements. You can adjust the wording and add it to the monitor description.

adyen/README.md Outdated
# Adyen

## Overview
[Adyen][1] is a global payment platform that provides a comprehensive suite of payment solutions for businesses. It enables companies to accept payments across online, mobile, and in-store channels. Adyen supports a wide range of payment methods, including credit cards, mobile wallets, and local payment options, and offers services such as fraud prevention and risk management.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[Adyen][1] is a global payment platform that provides a comprehensive suite of payment solutions for businesses. It enables companies to accept payments across online, mobile, and in-store channels. Adyen supports a wide range of payment methods, including credit cards, mobile wallets, and local payment options, and offers services such as fraud prevention and risk management.
[Adyen][1] is a global payment platform that provides a comprehensive suite of payment solutions for businesses. It enables companies to accept payments online, on mobile, and in-store. Adyen supports a wide range of payment methods, including credit cards, mobile wallets, and local payment options, and offers services such as fraud prevention and risk management.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done changes as per suggestions.

adyen/README.md Outdated
## Overview
[Adyen][1] is a global payment platform that provides a comprehensive suite of payment solutions for businesses. It enables companies to accept payments across online, mobile, and in-store channels. Adyen supports a wide range of payment methods, including credit cards, mobile wallets, and local payment options, and offers services such as fraud prevention and risk management.

The Adyen integration seamlessly collects the data of transactions, disputes, and payouts using the Adyen webhook capability and ingests them into Datadog for comprehensive analysis.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The Adyen integration seamlessly collects the data of transactions, disputes, and payouts using the Adyen webhook capability and ingests them into Datadog for comprehensive analysis.
The Adyen integration collects transaction, dispute, and payout data using Adyen's webhook capability and ingests it into Datadog for comprehensive analysis.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done changes as per suggestions.

adyen/README.md

Follow the instructions below to configure this integration for your Adyen account.

### Configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend bumping up Webhook Configuration one level:

Suggested change
### Configuration
### Webhook configuration

Copy link
Contributor

Choose a reason for hiding this comment

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

Done changes as per suggestions.

adyen/README.md

### Configuration

#### Webhook Configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

See recommendation to bump this out one level

Suggested change
#### Webhook Configuration

Copy link
Contributor

Choose a reason for hiding this comment

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

Done changes as per suggestions.

adyen/README.md Outdated
### Configuration

#### Webhook Configuration
Configure the Datadog endpoint to forward Adyen events as logs to Datadog. See [Adyen webhook overview][2] for more details.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Configure the Datadog endpoint to forward Adyen events as logs to Datadog. See [Adyen webhook overview][2] for more details.
Configure the Datadog endpoint to forward Adyen events as logs to Datadog. For more details, see [Adyen webhook overview][2].

Copy link
Contributor

Choose a reason for hiding this comment

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

Done changes as per suggestions.

adyen/assets/monitors/high_number_of_failed_payout_requests_detected.json Outdated
"created_at": "2024-09-26",
"last_updated_at": "2024-09-26",
"title": "High number of failed payout requests detected",
"description": "This monitor tracks and alerts when a high number of payout requests gets failed.",
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment on adding context

Copy link
Contributor

Choose a reason for hiding this comment

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

Done changes as per suggestions.

adyen/assets/monitors/high_number_of_failed_refunds_detected.json Outdated
"created_at": "2024-09-26",
"last_updated_at": "2024-09-26",
"title": "High number of failed refunds detected",
"description": "This monitor tracks and alerts when a high number of refunds gets failed.",
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment on adding context

Copy link
Contributor

Choose a reason for hiding this comment

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

Done changes as per suggestions.

adyen/assets/monitors/notification_of_chargeback.json Outdated
"created_at": "2024-09-26",
"last_updated_at": "2024-09-26",
"title": "Notification of Chargeback",
"description": "This monitor tracks and alerts when a chargeback is detected.",
Copy link
Contributor

Choose a reason for hiding this comment

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

See commend on adding context

Copy link
Contributor

Choose a reason for hiding this comment

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

Done changes as per suggestions.

adyen/assets/monitors/notification_of_fraud.json Outdated
"created_at": "2024-09-26",
"last_updated_at": "2024-09-26",
"title": "Notification of Fraud",
"description": "This monitor tracks and alerts when a fraud is detected.",
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment on adding context

Copy link
Contributor

Choose a reason for hiding this comment

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

Done changes as per suggestions.

adyen/assets/monitors/second_chargeback_detected.json Outdated
"created_at": "2024-09-26",
"last_updated_at": "2024-09-26",
"title": "Second Chargeback detected",
"description": "This monitor tracks and alerts when a second chargeback is detected.",
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment on adding context

Copy link
Contributor

Choose a reason for hiding this comment

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

Done changes as per suggestions.

@DhruvaPatel-crest DhruvaPatel-crest requested a review from a team as a code owner December 3, 2024 12:14
estherk15
estherk15 previously approved these changes Dec 4, 2024
View reviewed changes
gunterd
gunterd previously approved these changes Dec 5, 2024
View reviewed changes
@avaid-crest avaid-crest dismissed stale reviews from gunterd and estherk15 via c0b0244 December 11, 2024 11:23
brunorenier
brunorenier approved these changes Dec 19, 2024
View reviewed changes
Copy link

@brunorenier brunorenier left a comment

Choose a reason for hiding this comment

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

Reapproving for logs since the last review was stale and I don't see any changes.

@gunterd gunterd mentioned this pull request Jan 3, 2025
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DhruvaPatel-crest DhruvaPatel-crest DhruvaPatel-crest left review comments

@vrajpatel-crest vrajpatel-crest vrajpatel-crest left review comments

@brunorenier brunorenier brunorenier approved these changes

@estherk15 estherk15 estherk15 approved these changes

@gunterd gunterd gunterd approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
assets/deploy-logs-staging ONLY USED BY Logs Backend - Validates that a PR is OK to go to staging changelog/no-changelog crest/asset-pr dev/testing dev/tooling docs/approved documentation ecosystems/review-requested editorial review Waiting on a more in-depth review from a docs team editor product/review-requested qa/skip-qa Automatically skip this PR for the next QA team/agent-integrations team/documentation team/logs-backend team/siem-logs-reviewers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@avaid-crest @drichards-87 @gunterd @estherk15 @brunorenier @vrajpatel-crest @DhruvaPatel-crest @dj0well @nubtron