implementation to get dynamic config during runtime#7
implementation to get dynamic config during runtime#7shashank11p wants to merge 3 commits intomainfrom
Conversation
pavolloffay
left a comment
pavolloffay
left a comment
There was a problem hiding this comment.
Nice start, could we add a test to verify the behaviour? You can spin up a a mocked backed service and then get the config from it.
| configBuilder.setEnabled(response.getEnabled()); | ||
| configBuilder.setDataCapture(response.getDataCapture()); | ||
| configBuilder.setJavaagent(response.getJavaAgent()); | ||
| agentConfig = configBuilder.build(); |
There was a problem hiding this comment.
is this the field from the HypertraceConfig class?
There was a problem hiding this comment.
Should we worry about concurrent modifications (read/write) at the same time?
There was a problem hiding this comment.
Yes, this is the field from HypertraceConfig class. We do not need to worry about concurrent modifications as it is a volatile field.
|
|
||
| private final ConfigurationServiceGrpc.ConfigurationServiceBlockingStub blockingStub; | ||
|
|
||
| private static Int64Value configTimestamp; |
| DynamicConfigFetcher dynamicConfigFetcher = null; | ||
| if (dynamicConfigServiceUrl != null) { | ||
| dynamicConfigFetcher = new DynamicConfigFetcher(dynamicConfigServiceUrl); | ||
| Executors.newScheduledThreadPool(1) |
There was a problem hiding this comment.
Is there an executor service in the upstream that we could use? The upstream javaagent-tooling might have smth that we could reuse. We also have to set the thread to the daemon otherwise it will be blocking shutdown
There was a problem hiding this comment.
Upstream does not have any executor service that I could find. They have also used Executors directly https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetrics.java#L81
2 participants
Assuming grpc service will be similar to what is in hypertrace/agent-config#61.