connect opamp to dynamic config#656
Conversation
| if (!startOpAmp) { | ||
| return; | ||
| } | ||
| String serviceName = getServiceName(properties); |
There was a problem hiding this comment.
This won't take service names derived from ResourceProviders into account, will it?
If not, we should probably add a TODO here?
There was a problem hiding this comment.
It will if the resource provider sets the resource "service.name" entry before this executes (which should be the case) - is there another case that needs handling? I'm not sure about whether the resource providers can set it elsewhere
There was a problem hiding this comment.
I was asking because getServiceName(properties) only reads from the ConfigurationProperties and not potential changes from ResourceProviders which don't write those, but directly set the Resource, e.g. AppServiceNameProvider.
But maybe there is some magic behind the curtains making this work? So if you tested it this is fine, otherwise we should add a TODO
custom/src/main/java/co/elastic/otel/dynamicconfig/CentralConfig.java
Outdated
Show resolved
Hide resolved
… into connect-opamp-dynamic-config
2 participants
Mainly adding and activating a new CentralConfig class, plus a little refactoring to align method names to option names. No test of the CentralConfig class in this PR, I'll do that in a followup. This is almost the last PR in the sequence to get central config working and integrated to dynamic config, the only thing left is that test and some additional work around property/env var names for the new functionality