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

Add datadog plugin for tracing #5681

Merged

Conversation

kalfonso
Copy link
Contributor

Signed-off-by: Karel Alfonso Sague kalfonso@squareup.com

Signed-off-by: Karel Alfonso Sague <kalfonso@squareup.com>
@kalfonso kalfonso requested a review from sougou as a code owner January 10, 2020 03:08
@kalfonso
Copy link
Contributor Author

@systay you might be familiar with this change

@tirsen tirsen requested a review from systay January 12, 2020 22:25
@tirsen
Copy link
Collaborator

tirsen commented Jan 12, 2020

I think we've had different opinions on dependencies on these commercial third parties. That said DataDog is pretty popular and if you don't use it there is only a tiny bit of overhead for the dependency.

Copy link
Collaborator

@systay systay left a comment

Choose a reason for hiding this comment

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

LGTM

@deepthi
Copy link
Member

deepthi commented Jan 14, 2020

I think we've had different opinions on dependencies on these commercial third parties. That said DataDog is pretty popular and if you don't use it there is only a tiny bit of overhead for the dependency.

@kalfonso FTR, can you document the change in binary sizes /docker images here?
Also are there any license implications from the packages we are importing?

@derekperkins
Copy link
Member

What about hiding these behind build tags?

@sougou
Copy link
Contributor

sougou commented Jan 19, 2020

I took a look at the implementation. Looks lightweight, and it ironically depends on opentracing, which we already depend on.

So, lets move ahead with this. If this becomes a problem in the future, we can reconsider our options.

@sougou sougou merged commit c4e06a1 into vitessio:master Jan 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants