airbyte
airbyte copied to clipboard
✨ [source-datadog] refactor connector and add new stream `service_level_objectives_history`
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:
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 |
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 abreakingChanges
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,
-
Check for hidden checklists in your PR description
-
Toggle the github label
checklist-action-run
on/off to re-run the checklist CI.
Also worth noting that this depends on #35341
queries -> removed (it was not referenced anywhere in the manifest)
🤦🏼 Thank you for cleaning it up. We're taking a look.
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?
@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.
Ah no I'm wrong, it's actually not bad. But, we'd need to carefully resolve the conflicts.