airbyte icon indicating copy to clipboard operation
airbyte copied to clipboard

✨ [source-datadog] refactor connector and add new stream `service_level_objectives_history`

Open dlecocq opened this issue 1 year ago • 5 comments

What

At least of the options in the DataDog connector were overloaded to be used across multiple streams. In some cases, this was appropriate (for example, the maximum number of records retrieved in a single request), and in some cases not (for example, the "query", which may not be appropriate to use for both SLOs and logs (and other streams). In service of that, stream-specific configuration has been isolated to their own configuration groups, and imposing a naming convention (<stream>__<setting>) to differentiate service_level_objectives__query and logs__query.

In addition, not all of the options available on the DataDog endpoints were previously exposed. This MR adds several (for example, monitors__tags, monitors__with_downtimes, etc.), though even this change is not exhaustive. My hope is that if this convention suits the community, we would expand it to the remaining endpoints so that Airbyte exposes more to the connector user.

Lastly, this adds a new stream, the service level objectives history.

How

(Captured above)

🚨 User Impact 🚨

Breaking changes - some of the existing configuration options have been supplanted by their newer versions:

  • query -> logs__query
  • queries -> removed (it was not referenced anywhere in the manifest)

Version bump - an argument could be made for a major bump, but I would suggest a minor version bump to 0.5.0.

Community member or Airbyter

  • Community member? Grant edit access to maintainers
  • 🔴 Unit & integration tests added and passing I have not been able to get airbyte-ci to handle this connector well to run the tests, but I'm working on it. In the meantime, there's value in submitting the MR.

I'll also include some screenshots of it working in my local Airflow:

Screenshot 2024-02-16 at 11 54 13 AM Screenshot 2024-02-16 at 11 54 20 AM Screenshot 2024-02-16 at 11 54 27 AM

dlecocq avatar Feb 16 '24 18:02 dlecocq

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ❌ Failed (Inspect) Jun 14, 2024 2:31pm

vercel[bot] avatar Feb 16 '24 18:02 vercel[bot]

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

  • [ ] PR name follows PR naming conventions
  • [ ] Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
  • [ ] Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • [ ] You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
  • [ ] Secrets in the connector's spec are annotated with airbyte_secret
  • [ ] All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
  • [ ] Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
  • [ ] Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
  • [ ] If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

  1. Check for hidden checklists in your PR description

  2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.

github-actions[bot] avatar Feb 16 '24 18:02 github-actions[bot]

Also worth noting that this depends on #35341

dlecocq avatar Feb 16 '24 18:02 dlecocq

queries -> removed (it was not referenced anywhere in the manifest)

🤦🏼 Thank you for cleaning it up. We're taking a look.

natikgadzhi avatar Feb 16 '24 20:02 natikgadzhi

Once the CDK change gets merged in, let's run tests on this, and merge this.

@dlecocq, if you have a minute, would you be open to improving a few other things in source-datadog? Would it be okay if I reach out to you with those changes in a separate issue?

natikgadzhi avatar Mar 03 '24 22:03 natikgadzhi

@marcosmarxm heads up, this is very behind master. It might be easier to cherry-pick the changes and reapply them in a fresh PR, if @dlecocq has time.

natikgadzhi avatar Jun 12 '24 23:06 natikgadzhi

Ah no I'm wrong, it's actually not bad. But, we'd need to carefully resolve the conflicts.

natikgadzhi avatar Jun 12 '24 23:06 natikgadzhi

/format-fix

Format-fix job started... Check job output.

🟦 Job completed successfully (no changes).

marcosmarxm avatar Jun 14 '24 14:06 marcosmarxm