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

Initial commit of Ocient integration (ECOINT-49) #2546

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

Initial commit of Ocient integration (ECOINT-49) #2546

wants to merge 20 commits into from

Conversation

ocient-cliimatta
Copy link

@ocient-cliimatta ocient-cliimatta commented Nov 22, 2024 •
edited
Loading

What does this PR do?

This PR introduces the integration for the Ocient Data Warehouse.

Motivation

We're part of the Datadog partner program and would like to offer this integration to our customers to monitor their Ocient environments with Datadog.

Review checklist

  • PR has a meaningful title or PR has the no-changelog label attached
  • Feature or bugfix has tests
  • Git history is clean
  • If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo
  • If this PR includes a log pipeline, please add a description describing the remappers and processors.

Additional Notes

Anything else we should know when reviewing?

@ocient-cliimatta ocient-cliimatta changed the title Main ocientcheck Initial commit of Ocient integration Nov 25, 2024
@ocient-cliimatta ocient-cliimatta marked this pull request as ready for review November 25, 2024 19:47
@ocient-cliimatta ocient-cliimatta requested review from a team as code owners November 25, 2024 19:47
@estherk15 estherk15 added the editorial review Waiting on a more in-depth review from a docs team editor label Nov 25, 2024
estherk15
estherk15 reviewed Nov 26, 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.

Thanks for drafting this! Left a few suggestions, let me know if you have any questions!

ocient/README.md Outdated Show resolved Hide resolved
ocient/README.md Outdated Show resolved Hide resolved
ocient/README.md Outdated Show resolved Hide resolved
ocient/assets/dashboards/ocient_overview.json Outdated Show resolved Hide resolved
@ocient-cliimatta
Copy link
Author

@eho1307 @estherk15 please review my latest commit

estherk15
estherk15 approved these changes 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.

Looks good, thanks for making those updates!

@eho1307 eho1307 requested review from a team and james-eichelbaum and removed request for a team December 11, 2024 19:26
@dd-dominic dd-dominic changed the title Initial commit of Ocient integration Initial commit of Ocient integration (ECOINT-49) Dec 16, 2024
@eho1307
Copy link
Contributor

eho1307 commented Jan 7, 2025

Hi @ocient-cliimatta, Apologies as many of our teammates including myself were on vacation these last 2 weeks. Please expect a delay while we catch up. Appreciate your patience so far.

eho1307
eho1307 requested changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@eho1307 eho1307 left a comment

Choose a reason for hiding this comment

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

Thank you for opening this PR! Appreciate your patience while we're catching up after the holidays. Please let me know if you have any questions or concerns on my feedback below.

ocient/README.md

With industry-standard interfaces like SQL and JDBC, Ocient makes it easy for organizations to interact with data within its platform.
The Ocient integration enables you to collect metrics related to query performance, disk usage, database tables, and more.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add 1-2 sentences about what value this brings to our mutual customers?

ocient/assets/dashboards/ocient_overview.json
Copy link
Contributor

Choose a reason for hiding this comment

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

Please include:

  1. Logo at the top of your dashboard
  2. Link to the Ocient documentation you refer to in the widget on the top right
  3. Update the dashboard description. You can do so by hovering over the title and clicking edit. Remove the word 'cloned' as well

ocient/images/ocient_dashboard.png
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this dashboard different from the dashboard json you uploaded? It appears as though the logo and description are present here.

ocient/images/ocient_logo.png
Copy link
Contributor

Choose a reason for hiding this comment

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

Please replace this with an svg version of the logo

ocient/manifest.json
"changelog": "CHANGELOG.md",
"description": "Collect performance metrics and monitor overall Ocient cluster health",
"title": "Ocient",
"media": [],
Copy link
Contributor

Choose a reason for hiding this comment

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

Please reference the image of your dashboard here.

Suggested change
"media": [],
"media": [
"media_type": "image",
"caption": "[insert caption here]",
"image_url": "images/ocient_dashboard.png"
],

ocient/metadata.csv
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also fill in the unit_name column? It's best practice. You can leave a cell blank if there's no applicable unit_name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eho1307 eho1307 eho1307 requested changes

@HadhemiDD HadhemiDD HadhemiDD approved these changes

@estherk15 estherk15 estherk15 approved these changes

@james-eichelbaum james-eichelbaum Awaiting requested review from james-eichelbaum james-eichelbaum was automatically assigned from DataDog/ecosystems-review

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
agent/approved docs/approved ecosystems/review-requested editorial review Waiting on a more in-depth review from a docs team editor kind/technology-partner product/logos-requested product/requested-changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ocient-cliimatta @eho1307 @estherk15 @HadhemiDD @ewozniak-Ocient