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

Is there a way to get only some of the enhanced database reporting? #444

Closed
evanlucas opened this issue Mar 20, 2017 · 4 comments
Closed
Assignees
Labels
api: cloudtrace Issues related to the googleapis/cloud-trace-nodejs API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@evanlucas
Copy link

In the pg plugin, the query and the values are added to the span only if enhanced database reporting is enabled. (https://github.com/GoogleCloudPlatform/cloud-trace-nodejs/blob/master/src/plugins/plugin-pg.js#L36-L41). While we would like to have the queries reported, reporting the values and rows from the response could end up being a little too much information (think of changing a password or access token).

Is there a way around this without manually patching pg and disabling the experimental plugin?

Thanks!

@evanlucas evanlucas changed the title Is there a way to get only *some* of the enhanced database reporting? Is there a way to get only some of the enhanced database reporting? Mar 20, 2017
@matthewloring
Copy link
Contributor

There is not currently a way to print queries/values without database results. I see a few possible ways to remedy this:

  1. We could determine that queries are not sensitive and remove them from being gated behind enhancedDatabaseReporting. I think this may be too simplistic and will not cover similar use cases for non-database plugins.

  2. We could have a notion of sensitivity levels (similar to log levels) that would allow labels to be added at a particular privacy level. I'm not aware of other apis with this functionality so it could be unintuitive. It may also be restrictive to impose a global privacy level on all plugins (maybe you want queries and values for redis but not pg).

  3. We could accept a list of allowed labels at configuration time for each plugin and only report data associated with allowed labels. This would address the issue at hand but it could be difficult to determine the set of possible label names for a particular plugin. It would also be brittle to plugin changes that modify label names or add new ones.

  4. We could allow plugins to specify their own arbitrary configuration that users of the trace agent could provide when they specify plugins. This would complicate the configuration process but would offer the most flexibility.

Do you have any other ideas or thoughts on these approaches?

@evanlucas
Copy link
Author

Personally, I think 4 is going to be the way to go. We will always want errors, queries, and rowCount, but not parameters or output. I guess the hard part would be how to enable/change configuration at the environment level. Would that mean that they have to be passed to the plugin? Seems like that would be a backwards-incompatible change with how the plugin format is currently { name: file }?

@kjin
Copy link
Contributor

kjin commented Mar 24, 2017

I also like (4), because it opens the door to other fields that can passed to a single plugin. We discussed this in the past for limiting label sizes. While it would complicate the interface, I think we can avoid a backward-incompatible change by checking the value type (either string or now also object)? Which makes it, to me, the option with the smallest downside.

@JustinBeckwith JustinBeckwith added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. 🚨 This issue needs some love. labels Jun 8, 2018
@JustinBeckwith JustinBeckwith removed the 🚨 This issue needs some love. label Jun 25, 2018
@kjin
Copy link
Contributor

kjin commented Aug 6, 2018

#826 adds a getConfig option on the Tracer object which can allow plugins to access the entire, normalized config object. While this doesn't directly add support for multiple levels of database reporting, this makes it possible for a fork of a given plugin to read arbitrary values as set from the user-provided configuration object.

Apologies for the delay here -- we should have acted sooner on this.

@kjin kjin closed this as completed Aug 6, 2018
@google-cloud-label-sync google-cloud-label-sync bot added the api: cloudtrace Issues related to the googleapis/cloud-trace-nodejs API. label Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: cloudtrace Issues related to the googleapis/cloud-trace-nodejs API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

4 participants