rabbah commented on issue #2282: Distributed tracing support #2192 URL: https://github.com/apache/incubator-openwhisk/pull/2282#issuecomment-308475467 I already raised some objections to these changes. I'll summarize them: 1. the added overhead on the activation message in space means messages that were already working will no longer work in a production system for the same payload; this is a concern 2. will need a test to ensure that whatever the desired invariant are from the sampling is actually being captured 3. in addition to a test for the sampling itself, we need a test to confirm that when the sample rate is 0 (ie no tracing), the size of the activation message is not affected I have a general concern about accepting one-offs and deferring the refactoring. Who's committing to doing the refactoring? Further, as I noticed, in some deployments, this will not be enabled at all. So we are incurring additional overhead for some deployments without articulating the gains. I think we have to be extra vigilant on this point as we have more contributors now and more community partners and just today there was a discussion about just this PR and why the OpenTracing (http://opentracing.io/) is not used instead of a direct integration. I have only made a cursory pass over the code but if you want to pursue this in its current form, I think I and others will likely want to make a more critical pass over the actual code. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
With regards, Apache Git Services
