[File based config] Support for default profiler's log ingest URL in declarative config#2518
[File based config] Support for default profiler's log ingest URL in declarative config#2518robsunday merged 13 commits intosignalfx:mainfrom
Conversation
This reverts commit 480405a.
…guration.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com>
Minor logging improvements
… URL for declarative config
| + "You can override it by setting " | ||
| + CONFIG_KEY_INGEST_URL, | ||
| new Object[] {ingestUrl, defaultIngestUrl}); | ||
| return defaultIngestUrl; |
There was a problem hiding this comment.
[For reviewer] It now returns default ingest URL instead of null, which is more consistent with the warning reported above.
Configuration.getIngestUrl() method is called only from LogExporterBuilder which then calls appropriate log exporter builder: OtlpGrpcLogRecordExporterBuilder or OtlpHttpLogRecordExporterBuilder. These use their own default URLs in case endpoint is undefined, but they are basically identical to these returned from Configuration.getIngestUrl() now.
| ConfigProperties config, Supplier<OtlpGrpcLogRecordExporterBuilder> makeBuilder) { | ||
| OtlpGrpcLogRecordExporterBuilder builder = makeBuilder.get(); | ||
| String ingestUrl = Configuration.getConfigUrl(config); | ||
| if (ingestUrl != null) { |
There was a problem hiding this comment.
[for reviewer] No need for null check now
| builder::setRetryPolicy, | ||
| builder::setMemoryMode); | ||
|
|
||
| if (ingestUrl != null) { |
There was a problem hiding this comment.
[for reviewer] No need for null check now
robsunday
changed the title
robsunday
changed the title
| if (ingestUrl != null) { | ||
| builder.setEndpoint(ingestUrl); | ||
| } | ||
| String ingestUrl = Configuration.getIngestUrl(config); |
There was a problem hiding this comment.
Perhaps it would be better to pass the ingest url to this method instead of the config?
There was a problem hiding this comment.
Your suggestion looks good, however now we have consistent method signatures for buildHttpExporter and buildGrpcExporter methods which improves readability. I'll merge this PR and then probably make a followup.
2 participants
In declarative config there is no support for
otel.exporter.otlp.endpointandotel.exporter.otlp.protocolproperties.Until now
Configuration.getIngestUrlreturnednullfor declarative config in casesplunk.profiler.logs-endpointwas not defined.Now
Configuration.getIngestUrlreturns "http://localhost:4318/v1/logs" or "http://localhost:4317", depending on value of profiler otlp protocol (default is "http/protobuf")