census-instrumentation/opencensus-java

A stats collection and distributed tracing framework

opencensus-java's Issues

Document tags as printable ascii

Tags seems check length, presumably as a byte-length guard as opposed to a character length one. If they are the same, we're probably talking printable ascii. If that's the case, we should document somewhere that names and values must be printable ascii (else they will be replaced with '?' or such). Maybe on the Tag type or something that accepts it. If there are other rules, such as downcasing or case insensitive keys, we should document that in the same place as we document the constraints of character set and length.

Consider building instrumentation-java with Gradle.

It would be nice to only have to maintain one build file, instead of separate build files for Bazel and Maven. We would need to ensure that we can still build with Java 6, upload releases to Maven Central, build Protocol Buffers, and run static analysis and style checks with Gradle.

Add Javadocs for tag keys in RpcMeasureConstants.java.

The RPC Measures and Views already have description fields, but it would help to have more documentation for the TagKeys:

opencensus-java/core/src/main/java/io/opencensus/stats/RpcMeasureConstants.java

Line 25 in 4502e9b

public static final TagKey RPC_STATUS = TagKey.create("canonical_status");

Should we allow the Travis CHECK_GIT_HISTORY build job to fail the build?

The git history check is currently listed under allow_failures, so it can't fail the build. I put it there initially because I wanted to make sure that there weren't too many false positives. I was wondering if we should remove it from allow_failures now that we have been using it for a few weeks. It fails when a pull request contains merge commits.

Add code examples to the 'tags' package.

Build and run tests in 'core' directory with Maven.

We currently disable the tests in core/pom.xml because they create a circular dependency between the core and core_impl directories (https://github.com/google/instrumentation-java/blob/1435c7fdf4be28ef724350558be6984f39d386c8/core/pom.xml#L36). core_impl depends on core, and the core tests have a runtime dependency on the non-test code under core_impl. One solution is to move the tests to core_impl/.

Rename API artifact to census-api

census-core doesn't indicate the fact that it's pure API. Because grpc-core depends on the API artifact, people may mistake census-core as an implementation and raise concern over grpc-core's dependency on it.

Create no-op StatsContextFactory implementation for Stats.getStatsContextFactory()

In https://github.com/songy23/instrumentation-java/blob/master/core/src/main/java/com/google/instrumentation/stats/Stats.java:

If Provider.newInstance() returns a null StatsManager, we need to return a no-op StatsContextFactory, instead of just returning null.

Move stats tests that depend on 'core_impl' out of 'core'.

core_impl depends on core, and the core tests have a runtime dependency on the non-test code under core_impl, which is a circular dependency between directories. I think it would be better to put all core_impl tests in the core_impl directory. This issue was part of #147.

Update StatsSerializer.serialze() and deserialze() to use byte[] instead of Input/OutputStream

From our conversation in PR #208, gRPC team suggested to use byte[] instead of Input[Output]Stream. Consider updating the input and output format of serialize() and deserialize().

Census keys and values should not crop input

StringUtil seems to discard values that are longer than 255. This is error prone, because long paths of values can accidentally be trimmed to the same value. It would be better to just throw an exception up front than silently do the wrong thing.

The max length is already exposed in TagValue, so it is possible for clients to avoid this error safely.

StringUtil constructor is visible?

The constructor seems to be visible for being able to be thrown, but solely for the purpose of testing. This should be private, and the test should be removed.

The only valid reason I can think of for doing something like this is for code coverage, which can usually be fixed by adding a special comment.

Consider replace ByteArrayOutputStream and access buffer directly in StatsSerializer.serialize()

As @technicalYogi mentioned in PR #124, directly using ByteArrayOutputStream and call toByteArray() may not be very efficient, as it needs to copy memory to different location. Instead he/she suggests inheriting from ByteArrayOutputStream and access the buffer directly. May be worth considering.

CensusContext.serialize should accept a destination buffer

Current, CensusContext.serialize always returns a buffer, which creates excess garbage. A better interface would be to accept an existing buffer to write into. This would allow callers to recycle buffers. Also, it would allow the context to serialize into an existing buffer of other data, avoiding a copy.

Would a Zipkin contrib module be interesting?

I've had some informal interest in a Zipkin support for this. Such would make it easy to test, as Zipkin can even run as a junit rule. Also, it could help with sites who already run Zipkin (as traces could enter the same depot). Finally, it could help familiarize folks with this project.

Such an integration would likely be an exporter and possibly a pluggable means to control (or append) trace identifiers in B3 format. The latter could be helpful as existing gRPC integration is done via B3 headers. Zipkin support should plug-in nicely to existing hooks in this project.

Generally speaking, I don't work until users are interested in it. If you are interested in this, please click thumbsup or otherwise note how it would be helpful to you. If you think this shouldn't be here, or shouldn't exist at all, you can also mention this.

Consider providing a no-op CensusContextFactory

There will be cases where a Census-instrumented product would want to disable Census, or need a place-holder when Census.getCenusContextFactory() returns null. Yes they could implement their own no-op CensusContextFactory, but since this may be a common use case, it makes sense that Census provides a no-op impl as part of the census-api artfiact.

Integrate with gRPC Context

Currently CensusContext.setCurrent() sets itself to the thread-local. I think it should attach to the current gRPC context instead. Perhaps it needs a new name. How about this:

abstract class CensusContext {
  // Returns a new gRPC Context based on current Context, with the CensusContext attach to it
  Context attach();
}

getCurrent() would be changed to retrieve from current gRPC Context, instead of from thread-local.

The typical usage on the server-side would be:

CensusContext cc = censusContextFactory.deserialize(censusContextBytes);
Context ctx = cc.attach();
executor.execute(new ContextRunnable(ctx) {
  // In this method, current Context is "ctx".
  @Override
  public void runInContext() {
    // run the RPC handler
    ...
    // at the end of the RPC
    CensusContext cc = censusContextFactory.getCurrent();  // retrives from current Context
    cc.record(...);
  }
});

Document the behavior for Scope.close().

See #446 (comment).

Inconsistent time units in RpcConstants

RPC_CLIENT_SERVER_ELAPSED_TIME, RPC_SERVER_SERVER_ELAPSED_TIME and RPC_SERVER_SERVER_LATENCY are documented to be in msec, which is milliseconds.

RPC_CLIENT_ROUNDTRIP_LATENCY is documented to be RPC roundtrip latency us, which is microseconds. There is no reason for it to be different.

Add check for license presence.

Configure to run tests and build on osx and windows.

We need to configure travis to build and run tests on osx.
We need to find a way to run our build/tests on windows as well.

Non symmetric API?

The serialization and deserialization of a Census Context seems to be split across two classes instead of one: CensusContextFactory.deserialize and CensusContext.serialize. Why have them in separate places?

Add a method to check whether a StatsContext serializes to nothing.

In https://github.com/grpc/grpc-java/blob/6ff5f2149bb643fa6cc951e42831c067003ff810/core/src/main/java/io/grpc/internal/StatsTraceContext.java#L140

We unconditionally put the stats context header in the metadata. For many uses, this header is completely empty, and results in unnecessary allocation and processing on the Critical path for RPCs. It would be really useful to add a method to StatsContext that indicates whether there is any data to serialize, and possibly skip adding the header. Perhaps hasSerializableData ?

are TagKey and TagValue idiomatic?

I can see the desire to solve the problem of length constraints with types, but in java, you can't extend String. Making TagKey and TagValue wrapper types makes it clunky to specify tags, in a way that will be even more obvious when other JVM languages use this. TagKey and TagValue feel clunkier than the Tag object itself.

Is there some other reason besides length we are doing this? If not, I'd highly recommend reverting and moving the length constraint to a precondition. Libraries should be a joy to use.

Define a gRPC Context key for StatsContext

Something similar to GrpcTraceUtils. This is needed for communicating StatsContext between the application and the gRPC core.

The StatsRunner example should be using this instead of defining its own key.

CensusContextFactory.deserialize should clarify mutabilty

CensusContextFactory.deserialize doesn't say whether it will mutate the passed in ByteBuffer. This means that all callers will have to make a defensive copy to avoid accidentally having the underlying data, mark, position, or limit values changed.

It should be clear from the documentation that it will be treated as immutable.

Update pom.xml and create a script to deploy to Maven Central.

Split RpcConstants into two files.

gRPC core only uses the MeasurementDescriptors, not the ViewDescriptors. Having all of them in RpcConstants forces JVM to load all related classes when loading RpcConstants, even if they are not used. Android is sensitive to class loading time. We'd better put the ViewDescriptors in a separate file.

Consider enabling Travis CI's Auto Cancellation feature.

https://blog.travis-ci.com/2017-03-22-introducing-auto-cancellation

I'm not sure if our build is long enough for us to need this feature, but it could be useful when making multiple changes to a PR.

Avoid sharing Java packages between API and implementation libraries.

Using different packages for implementation classes will make it easier to avoid accessing non-public elements in the API library and help maintain backwards compatibility.

New Gradle warning about output directory in Travis and AppVeyor builds.

Recent builds showing the warning:

AppVeyor: https://ci.appveyor.com/project/instrumentationjavateam/opencensus-java/branch/master https://ci.appveyor.com/project/instrumentationjavateam/opencensus-java/build/1.0.235
Travis (in all 3 Gradle builds): https://travis-ci.org/census-instrumentation/opencensus-java/builds/252129158

$ case "$TASK" in "CHECK_GIT_HISTORY") python check-git-history.py ;; "BUILD") export JAVA8_HOME=`jdk_switcher home oraclejdk8` ; ./gradlew clean assemble --stacktrace ; case "$TRAVIS_JDK_VERSION" in "oraclejdk8") ./gradlew check :all:jacocoTestReport ;; "oraclejdk7") ./gradlew check ;; esac ;; *) echo "Unknown task $TASK" ; exit 1 ;; esac
Starting a Gradle Daemon, 1 incompatible and 1 stopped Daemons could not be reused, use --status for details
Gradle now uses separate output directories for each JVM language, but this build assumes a single directory for all classes from a source set. This behaviour has been deprecated and is scheduled to be removed in Gradle 5.0
	at org.gradle.api.internal.tasks.DefaultSourceSetOutput.getClassesDir(DefaultSourceSetOutput.java:79)
	at org.gradle.api.internal.tasks.DefaultSourceSetOutput_Decorated.getClassesDir(Unknown Source)
	at me.tatarka.RetrolambdaPluginJava$_apply_closure1$_closure2.doCall(RetrolambdaPluginJava.groovy:43)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:483)
	at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
	at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
	at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:294)
	at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1022)
	at groovy.lang.Closure.call(Closure.java:414)
	at groovy.lang.Closure.call(Closure.java:430)
	at org.gradle.api.internal.ClosureBackedAction.execute(ClosureBackedAction.java:71)
	at org.gradle.util.ConfigureUtil.configureTarget(ConfigureUtil.java:160)
	at org.gradle.util.ConfigureUtil.configure(ConfigureUtil.java:106)
	at org.gradle.util.ConfigureUtil$1.execute(ConfigureUtil.java:123)
	at org.gradle.api.internal.DefaultDomainObjectCollection.all(DefaultDomainObjectCollection.java:136)
	at org.gradle.api.internal.DefaultDomainObjectCollection.all(DefaultDomainObjectCollection.java:154)
	at me.tatarka.RetrolambdaPluginJava$_apply_closure1.doCall(RetrolambdaPluginJava.groovy:39)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:483)
	at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
	at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
	at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:294)
	at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1022)
	at groovy.lang.Closure.call(Closure.java:414)
	at org.gradle.listener.ClosureBackedMethodInvocationDispatch.dispatch(ClosureBackedMethodInvocationDispatch.java:40)
	at org.gradle.listener.ClosureBackedMethodInvocationDispatch.dispatch(ClosureBackedMethodInvocationDispatch.java:25)
	at org.gradle.internal.event.AbstractBroadcastDispatch.dispatch(AbstractBroadcastDispatch.java:42)
	at org.gradle.internal.event.BroadcastDispatch$SingletonDispatch.dispatch(BroadcastDispatch.java:230)
	at org.gradle.internal.event.BroadcastDispatch$SingletonDispatch.dispatch(BroadcastDispatch.java:149)
	at org.gradle.internal.event.AbstractBroadcastDispatch.dispatch(AbstractBroadcastDispatch.java:58)
	at org.gradle.internal.event.BroadcastDispatch$CompositeDispatch.dispatch(BroadcastDispatch.java:324)
	at org.gradle.internal.event.BroadcastDispatch$CompositeDispatch.dispatch(BroadcastDispatch.java:234)
	at org.gradle.internal.event.ListenerBroadcast.dispatch(ListenerBroadcast.java:140)
	at org.gradle.internal.event.ListenerBroadcast.dispatch(ListenerBroadcast.java:37)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:93)
	at com.sun.proxy.$Proxy23.afterEvaluate(Unknown Source)
	at org.gradle.configuration.project.LifecycleProjectEvaluator.notifyAfterEvaluate(LifecycleProjectEvaluator.java:75)
	at org.gradle.configuration.project.LifecycleProjectEvaluator.doConfigure(LifecycleProjectEvaluator.java:69)
	at org.gradle.configuration.project.LifecycleProjectEvaluator.access$100(LifecycleProjectEvaluator.java:33)
	at org.gradle.configuration.project.LifecycleProjectEvaluator$ConfigureProject.run(LifecycleProjectEvaluator.java:103)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:317)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:309)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:185)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:95)
	at org.gradle.configuration.project.LifecycleProjectEvaluator.evaluate(LifecycleProjectEvaluator.java:49)
	at org.gradle.api.internal.project.DefaultProject.evaluate(DefaultProject.java:654)
	at org.gradle.api.internal.project.DefaultProject.evaluationDependsOn(DefaultProject.java:738)
	at org.gradle.api.internal.project.DefaultProject.evaluationDependsOn(DefaultProject.java:730)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:483)
	at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
	at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
	at org.gradle.internal.metaobject.BeanDynamicObject$MetaClassAdapter.invokeMethod(BeanDynamicObject.java:479)
	at org.gradle.internal.metaobject.BeanDynamicObject.tryInvokeMethod(BeanDynamicObject.java:191)
	at org.gradle.internal.metaobject.CompositeDynamicObject.tryInvokeMethod(CompositeDynamicObject.java:98)
	at org.gradle.internal.metaobject.MixInClosurePropertiesAsMethodsDynamicObject.tryInvokeMethod(MixInClosurePropertiesAsMethodsDynamicObject.java:30)
	at org.gradle.groovy.scripts.BasicScript$ScriptDynamicObject.tryInvokeMethod(BasicScript.java:134)
	at org.gradle.internal.metaobject.AbstractDynamicObject.invokeMethod(AbstractDynamicObject.java:160)
	at org.gradle.groovy.scripts.BasicScript.invokeMethod(BasicScript.java:83)
	at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.callCurrent(PogoMetaClassSite.java:75)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:166)
	at build_3w74iljmo4da3nitl8mayqaj6.run(/home/travis/build/census-instrumentation/opencensus-java/all/build.gradle:29)
	at org.gradle.groovy.scripts.internal.DefaultScriptRunnerFactory$ScriptRunnerImpl.run(DefaultScriptRunnerFactory.java:90)
	at org.gradle.configuration.DefaultScriptPluginFactory$ScriptPluginImpl$2.run(DefaultScriptPluginFactory.java:187)
	at org.gradle.configuration.ProjectScriptTarget.addConfiguration(ProjectScriptTarget.java:77)
	at org.gradle.configuration.DefaultScriptPluginFactory$ScriptPluginImpl.apply(DefaultScriptPluginFactory.java:192)
	at org.gradle.configuration.BuildOperationScriptPlugin$1.run(BuildOperationScriptPlugin.java:61)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:317)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:309)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:185)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:95)
	at org.gradle.configuration.BuildOperationScriptPlugin.apply(BuildOperationScriptPlugin.java:58)
	at org.gradle.configuration.project.BuildScriptProcessor.execute(BuildScriptProcessor.java:39)
	at org.gradle.configuration.project.BuildScriptProcessor.execute(BuildScriptProcessor.java:26)
	at org.gradle.configuration.project.ConfigureActionsProjectEvaluator.evaluate(ConfigureActionsProjectEvaluator.java:34)
	at org.gradle.configuration.project.LifecycleProjectEvaluator.doConfigure(LifecycleProjectEvaluator.java:63)
	at org.gradle.configuration.project.LifecycleProjectEvaluator.access$100(LifecycleProjectEvaluator.java:33)
	at org.gradle.configuration.project.LifecycleProjectEvaluator$ConfigureProject.run(LifecycleProjectEvaluator.java:103)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:317)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:309)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:185)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:95)
	at org.gradle.configuration.project.LifecycleProjectEvaluator.evaluate(LifecycleProjectEvaluator.java:49)
	at org.gradle.api.internal.project.DefaultProject.evaluate(DefaultProject.java:654)
	at org.gradle.api.internal.project.DefaultProject.evaluate(DefaultProject.java:132)
	at org.gradle.execution.TaskPathProjectEvaluator.configure(TaskPathProjectEvaluator.java:35)
	at org.gradle.execution.TaskPathProjectEvaluator.configureHierarchy(TaskPathProjectEvaluator.java:62)
	at org.gradle.configuration.DefaultBuildConfigurer.configure(DefaultBuildConfigurer.java:38)
	at org.gradle.initialization.DefaultGradleLauncher$ConfigureBuild.run(DefaultGradleLauncher.java:195)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:317)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:309)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:185)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:95)
	at org.gradle.initialization.DefaultGradleLauncher.doBuildStages(DefaultGradleLauncher.java:157)
	at org.gradle.initialization.DefaultGradleLauncher.access$200(DefaultGradleLauncher.java:44)
	at org.gradle.initialization.DefaultGradleLauncher$1.run(DefaultGradleLauncher.java:122)
	at org.gradle.internal.work.DefaultWorkerLeaseService$2.execute(DefaultWorkerLeaseService.java:124)
	at org.gradle.initialization.DefaultGradleLauncher.doBuild(DefaultGradleLauncher.java:116)
	at org.gradle.initialization.DefaultGradleLauncher.run(DefaultGradleLauncher.java:99)
	at org.gradle.launcher.exec.GradleBuildController.run(GradleBuildController.java:66)
	at org.gradle.tooling.internal.provider.ExecuteBuildActionRunner.run(ExecuteBuildActionRunner.java:28)
	at org.gradle.launcher.exec.ChainingBuildActionRunner.run(ChainingBuildActionRunner.java:35)
	at org.gradle.tooling.internal.provider.ValidatingBuildActionRunner.run(ValidatingBuildActionRunner.java:32)
	at org.gradle.launcher.exec.RunAsBuildOperationBuildActionRunner$1.run(RunAsBuildOperationBuildActionRunner.java:43)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:317)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:309)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:185)
	at org.gradle.internal.progress.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:95)
	at org.gradle.launcher.exec.RunAsBuildOperationBuildActionRunner.run(RunAsBuildOperationBuildActionRunner.java:40)
	at org.gradle.tooling.internal.provider.SubscribableBuildActionRunner.run(SubscribableBuildActionRunner.java:51)
	at org.gradle.launcher.exec.InProcessBuildActionExecuter.execute(InProcessBuildActionExecuter.java:44)
	at org.gradle.launcher.exec.InProcessBuildActionExecuter.execute(InProcessBuildActionExecuter.java:28)
	at org.gradle.tooling.internal.provider.ContinuousBuildActionExecuter.execute(ContinuousBuildActionExecuter.java:75)
	at org.gradle.tooling.internal.provider.ContinuousBuildActionExecuter.execute(ContinuousBuildActionExecuter.java:49)
	at org.gradle.tooling.internal.provider.ServicesSetupBuildActionExecuter.execute(ServicesSetupBuildActionExecuter.java:51)
	at org.gradle.tooling.internal.provider.ServicesSetupBuildActionExecuter.execute(ServicesSetupBuildActionExecuter.java:32)
	at org.gradle.tooling.internal.provider.GradleThreadBuildActionExecuter.execute(GradleThreadBuildActionExecuter.java:36)
	at org.gradle.tooling.internal.provider.GradleThreadBuildActionExecuter.execute(GradleThreadBuildActionExecuter.java:25)
	at org.gradle.tooling.internal.provider.StartParamsValidatingActionExecuter.execute(StartParamsValidatingActionExecuter.java:64)
	at org.gradle.tooling.internal.provider.StartParamsValidatingActionExecuter.execute(StartParamsValidatingActionExecuter.java:29)
	at org.gradle.tooling.internal.provider.SessionFailureReportingActionExecuter.execute(SessionFailureReportingActionExecuter.java:55)
	at org.gradle.tooling.internal.provider.SessionFailureReportingActionExecuter.execute(SessionFailureReportingActionExecuter.java:42)
	at org.gradle.tooling.internal.provider.SetupLoggingActionExecuter.execute(SetupLoggingActionExecuter.java:50)
	at org.gradle.tooling.internal.provider.SetupLoggingActionExecuter.execute(SetupLoggingActionExecuter.java:30)
	at org.gradle.launcher.daemon.server.exec.ExecuteBuild.doBuild(ExecuteBuild.java:67)
	at org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:36)
	at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
	at org.gradle.launcher.daemon.server.exec.WatchForDisconnection.execute(WatchForDisconnection.java:37)
	at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
	at org.gradle.launcher.daemon.server.exec.ResetDeprecationLogger.execute(ResetDeprecationLogger.java:26)
	at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
	at org.gradle.launcher.daemon.server.exec.RequestStopIfSingleUsedDaemon.execute(RequestStopIfSingleUsedDaemon.java:34)
	at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
	at org.gradle.launcher.daemon.server.exec.ForwardClientInput$2.call(ForwardClientInput.java:74)
	at org.gradle.launcher.daemon.server.exec.ForwardClientInput$2.call(ForwardClientInput.java:72)
	at org.gradle.util.Swapper.swap(Swapper.java:38)
	at org.gradle.launcher.daemon.server.exec.ForwardClientInput.execute(ForwardClientInput.java:72)
	at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
	at org.gradle.launcher.daemon.server.exec.LogAndCheckHealth.execute(LogAndCheckHealth.java:55)
	at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
	at org.gradle.launcher.daemon.server.exec.LogToClient.doBuild(LogToClient.java:62)
	at org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:36)
	at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
	at org.gradle.launcher.daemon.server.exec.EstablishBuildEnvironment.doBuild(EstablishBuildEnvironment.java:72)
	at org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:36)
	at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:120)
	at org.gradle.launcher.daemon.server.exec.StartBuildOrRespondWithBusy$1.run(StartBuildOrRespondWithBusy.java:50)
	at org.gradle.launcher.daemon.server.DaemonStateCoordinator$1.run(DaemonStateCoordinator.java:297)
	at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:63)
	at org.gradle.internal.concurrent.StoppableExecutorImpl$1.run(StoppableExecutorImpl.java:46)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:55)
	at java.lang.Thread.run(Thread.java:745)
:all:clean UP-TO-DATE

/cc @bogdandrutu

CensusGrpcContext API semantics

getInstace() will return null if the implementation is not found. It should be annotated with @Nullable.

get()'s javadoc says

If there is no associated CensusContext, the default CensusContext is returned.

However, because only CensusContextFactory can get the default CensusContext, CensusGrpcContext has no access to the currently used CensusContextFactory implementation. Returning null is very unfriendly to users. Maybe change it to:

get(Context context, CensusContextFactory censusFactory)

Remove proto directory

After we migrate to use the new encoding, we are not depending on proto now. The proto directory can be safely removed.

Remove Bazel build files, and only build with Maven.

Maintaining two builds is time-consuming. Since Bazel currently doesn't support deploying to Maven Central or running different static analysis tools, we should use Maven for now. We will need to fix #147 and possibly add Error Prone to the Maven build first.

Remove Guava and jsr305 dependencies from the API artifact

They are only lightly used and can be easily swapped out. A dependency-free library is more appealing to users. In particular, users are concerned about Guava dependency because they may also be using Guava, and the two Guava versions may conflict.

Consider generating value classes with AutoValue.

We could probably generate a significant number of the classes under core/java/com/google/instrumentation/ using AutoValue. It would reduce boilerplate, but it might complicate the build.

Fix a flaky test in StatsContextTest.java

https://github.com/google/instrumentation-java/blob/master/core_impl/src/test/java/com/google/instrumentation/stats/StatsContextTest.java#L126

This test is flaky because we are storing the tags in a Map, and the order of tags could be random on different machines. Yet we're comparing the expected and actual serialized tags under the assumption that they have the same order. Needs to figure out a better way for this test.

Leading / Trailing whitespace in StringUtil

StringUtil.sanitize appears to try to clean up input strings, but often leading and trailing whitespace should be removed. This is especially true for code that is going to go into HTTP headers.

Rename the repository to opencensus-java

TagValue API

Looking through the TagValue class I see some possible performance problems.

First., the TagValue constructor is public, which means it isn't possible to control access to it. This is Item #1 in Effective Java. Any sort of interesting caching behavior will not be possible.

Second, it delegates to StringUtil.sanitize. This method is expensive to call and will hurt performance when being called rapidly. It will create a lot of garbage by rebuilding the String character by character. Users of this will not be able to skip this expensive setup every time. gRPC for example, already limits the possible characters in a lot of places and doesn't need this extra validation.

Third, it doesn't check for Null values. Are null values allowed? It seems like it will fail when doing equals or hashCode, but isn't clear that any other part of TagValue will break. Without any javadoc on the constructor, how will users know what values are valid?

Build artifacts compatible with Java 6

gRPC runs with as old as Java 6. Census would need to build artifacts that are compatible with Java 6 in order to be used by gRPC.

io.grpc.Context is a strict dependency, but it isn't strictly for grpc

in #154 io.grpc.Context was made a strict dependency of this project. In fact, it is a utility type which doesn't need to be branded as grpc (and could possibly be more re-used if it wasn't). For example, if context was a standalone library, it could use semantic versioning, which could help other projects use it. It could also help this project achieve a better sense of independence from grpc (which is helpful if it wants to be used in competing frameworks)

Fix potential overflow bug with ByteBuffer in StatsSerializer.java

https://github.com/google/instrumentation-java/blob/master/core_impl/src/main/java/com/google/instrumentation/stats/StatsSerializer.java#L51

Currently for the StatsSerializer.serialize() method, we're allocating a ByteBuffer with an initial capacity of 1024 bytes. However ByteBuffer cannot automatically resize, so if > 1024 bytes were appended to the ByteBuffer, an exception will be thrown. Need to fix this bug before next release.

MetricMap should be final

Despite the constructor being private, Mocking and other frameworks will just ignore this and create wrong instances anyways.

Replace "Census" with "Instrumentation" throughout the repository.

We still use "Census" in a few places, such as the readme and pom.xml files. The pom.xml change is necessary for #149.

Rename StatsManager to match naming in the trace package.

The corresponding class for tracing is TraceComponent, so StatsComponent would be more consistent.

Change CensusContext to abstract class

The with() family methods delegate to builder(), thus should be provided by the abstract class, and they should be final too.

Abstract class also gives us freedom of adding new methods in the future without breaking existing implementations.

Update 'stats' package to use new Provider API.

We should update all code that calls Provider.newInstance to call Provider.createInstance instead. createInstance works well with shading and Android.

newInstance:
https://github.com/google/instrumentation-java/blob/1435c7fdf4be28ef724350558be6984f39d386c8/core/src/main/java/com/google/instrumentation/common/Provider.java#L39

createInstance:
https://github.com/google/instrumentation-java/blob/1435c7fdf4be28ef724350558be6984f39d386c8/core/src/main/java/com/google/instrumentation/common/Provider.java#L65

Run Checker Framework null analysis as part of the Gradle build.

See https://checkerframework.org/manual/#nullness-checker. Null analysis would help detect bugs and also check the Nullable annotations that are part of the API. There are instructions for using Gradle: https://checkerframework.org/manual/#gradle

Spans with only RECORD_EVENTS option are also exported.

Please answer these questions before submitting a bug report.

What version of OpenCensus are you using?

0.5.1, 0.6.0-SNAPSHOT

What JVM are you using (java -version)?

java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)

What did you do?

If possible, provide a recipe for reproducing the error.
Setup RECORD_EVENTS and NeverSampler sampler when create a span. This span will get exported.

What did you expect to see?

No exported data for spans that are not sampled.

What did you see instead?

Exported data for spans that are not sampled.