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

Feature request: Configurable query comments #1643

Closed
jthandy opened this issue Jul 30, 2019 · 3 comments · Fixed by #1864
Closed

Feature request: Configurable query comments #1643

jthandy opened this issue Jul 30, 2019 · 3 comments · Fixed by #1864
Labels
enhancement New feature or request

Comments

@jthandy
Copy link
Member

jthandy commented Jul 30, 2019

Feature

Feature description

It is common to want to parse SQL statements run against a warehouse programmatically to determine information about who / what is running them. Different tools have different mechanisms for how they want their data to be passed, but essentially all of them are in some particular data format encapsulated in a comment block.

Here is an example of how Looker implements this feature:
https://docs.looker.com/admin-options/server/usage#sql_comments

Here is an example of how Intermix requests data from its upstream integrations:
https://docs.intermix.io/hc/en-us/articles/360004361073-intermix-io-App-Tracing-Guide

I do not have strong opinions on exactly how this should be implemented. My desired user experience is that I would set these query comments as a config parameter somewhere and then have dbt automatically add them to all SQL it runs on my behalf.

Who will this benefit?

Any user who has a dbt project of sufficient complexity that they need to perform "APM"-like tasks on top of it: monitoring warehouse usage, performance, reliability, responsiveness, cost, etc.

@jthandy
Copy link
Member Author

jthandy commented Jul 30, 2019

/cc @paul94133

@drewbanin
Copy link
Contributor

drewbanin commented Aug 16, 2019

By default, dbt should send the following payload:

{
    "app": "dbt",
    "dbt_version": "0.14.1a1",
    "target_name": "dev",
    "user": "drew",
    "file": "models/my_model.sql",
    "node_id": "model.my_new_package.my_model",
    "node_name": "incr",
    "resource_type": "model",
    "package_name": "my_new_package",
    "tags": [],
    "relation": "analytics.dbt_dbanin.my_model",
    "database": "analytics",
    "schema": "dbt_dbanin",
    "identifier": "my_model"
}

Some of these fields might not make sense together -- a test node isn't going to have a relation associated with it, for instance. Let's roll with this approach for now - we can always make the payload more contextually aware in the future.

@paul94133
Copy link

Request to add a property app = dbt to easily identify that these are coming from DBT.

@drewbanin drewbanin added this to the Louisa May Alcott milestone Aug 27, 2019
@drewbanin drewbanin added the enhancement New feature or request label Aug 27, 2019
beckjake added a commit that referenced this issue Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants