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.

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

Datadog application monitoring #100

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion hq_superset/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
OAuth2TokenMixin,
)
from cryptography.fernet import MultiFernet
from datadog import statsd
from superset import db
from superset_config import SKIP_DATASET_CHANGE_FOR_DOMAINS
from superset_config import SKIP_DATASET_CHANGE_FOR_DOMAINS, SERVER_ENVIRONMENT

from hq_superset.const import OAUTH2_DATABASE_NAME
from hq_superset.exceptions import TableMissing
Expand All @@ -34,6 +35,11 @@ class DataSetChange:
data: list[dict[str, Any]]

def update_dataset(self):
env_tag = f"env:{SERVER_ENVIRONMENT}"
with statsd.timed('cca.dataset_change.timer', tags=[env_tag, f"datasource:{self.data_source_id}"]):
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add domain tag ? I guess that would be useful?

Will datasource show on the metric itself and env would be on the top in the dropdown? and then if we add domain it would also be on the top of the dashboard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding the domain could be useful I suppose.

Will datasource show on the metric itself and env would be on the top in the dropdown?

What do you mean here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know how tags are used/available on datadog once they are added here in metrics.
For now, I have seen "env" or "domain" being available on top of a datadog dashboard (like here) and then additionally there are metrics available for different items within a graph (like here)
So, I was asking if the domain and data source ids will be available like so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see. I created a chart here to demonstrate that we can, yes.

(I'm not 100% sure if the timeseries is what we want long term, maybe the Top List chart works better?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Love it, thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

timeseries seems okay from the little I know, it does give me the data I think I want.

self._update_dataset()

def _update_dataset(self):
"""
Updates a dataset with ``self.data``.

Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
'Werkzeug==2.3.3',
'WTForms==2.3.3',
'ddtrace==2.17.0rc2',
'datadog==0.50.2',
],
extras_require={
'dev': [
Expand Down
2 changes: 2 additions & 0 deletions superset_config.example.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,3 +191,5 @@ class CeleryConfig:

USER_DOMAIN_ROLE_EXPIRY = 60 # minutes
SKIP_DATASET_CHANGE_FOR_DOMAINS = []

SERVER_ENVIRONMENT = 'changeme' # staging, production, etc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming this would be set up by ansible ?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍