New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Elastic registry: allow index name without date suffix #2705
Comments
If I understand your request correctly, I think you should be able to do this by setting the String index = "metrics";
String indexDateSeparator = "";
String dateFormat = "";
DateTimeFormatter indexDateFormatter = DateTimeFormatter.ofPattern(dateFormat);
ZonedDateTime dt = ZonedDateTime.ofInstant(Instant.now(), ZoneOffset.UTC);
System.out.println(index + indexDateSeparator + indexDateFormatter.format(dt)); // prints metrics |
Thanks for the suggestion - it does actually work! My concern with this approach is it seems unsupported. If the property comes in as null vs blank, or someone adds a different default in the future - then the date will revert back to the normal month based approach.
|
@bodin thank you for raising the issue. For some history, see #1331 which added an option to override the index naming entirely, but it does require extending the
I agree. And I think the reason overriding the index entirely was left as a more difficult exercise than an additional method in So I think there are a couple things we want to work on. We should figure out how index management will be for a majority of Micrometer/Elasticsearch users going forward and if that warrants a change in our defaults/suggested way of shipping metrics to elasticsearch. |
Thanks for the feedback - I did indeed end up overriding that method to customize the index name, cool to see that was a purposeful change. I opened the ticket because it was the only customization I was not able to do through existing property based configuration. I definitely agree that date based suffixes was the gold standard for time based data in the past, and I still have a few usecases still using that method. For myself at least, Datastreams are a nice replacement for the manual work I was doing maintaining the indexes and still produce the underlying date based naming convention on the actual indexes they create https://www.elastic.co/guide/en/elasticsearch/reference/current/data-streams.html Going forward, I am happy using the methods described in this thread and if there is a plan to revisit the default names in a future version, then documentation and test cases for the current 'hack' may not even be needed since it could change anyway. Thanks all for the discussion! |
I am also currently facing an issue with integrating micrometer with the data streams. I want to point out, that allowing for an empty index date suffix will not be enough to fix this issue. From data streams documentation:
it seems that as for now, the ElasticRegistry supports only private final String indexLine;
[...]
protected ElasticMeterRegistry(ElasticConfig config, Clock clock, ThreadFactory threadFactory, HttpSender httpClient) {
super(config, clock);
config().namingConvention(new ElasticNamingConvention());
this.config = config;
indexDateFormatter = DateTimeFormatter.ofPattern(config.indexDateFormat());
this.httpClient = httpClient;
if (StringUtils.isNotEmpty(config.pipeline())) {
indexLine = "{ \"index\" : {\"pipeline\":\"" + config.pipeline() + "\"} }\n";
} else {
indexLine = "{ \"index\" : {} }\n";
}
start(threadFactory);
}
[...]
String writeDocument(Meter meter, Consumer<StringBuilder> consumer) {
StringBuilder sb = new StringBuilder(indexLine);
String timestamp = generateTimestamp();
String name = getConventionName(meter.getId());
String type = meter.getId().getType().toString().toLowerCase();
sb.append("{\"").append(config.timestampFieldName()).append("\":\"").append(timestamp).append('"')
.append(",\"name\":\"").append(escapeJson(name)).append('"')
.append(",\"type\":\"").append(type).append('"');
List<Tag> tags = getConventionTags(meter.getId());
for (Tag tag : tags) {
sb.append(",\"").append(escapeJson(tag.getKey())).append("\":\"")
.append(escapeJson(tag.getValue())).append('"');
}
consumer.accept(sb);
sb.append('}');
return sb.toString();
} Error from elasticsearch api:
|
Please describe the feature request.
I would like the elastic registry to support a plain index name without injecting the month based suffix. Currently the only option is to provide the date format and the index prefix. It assumes micrometer will be managing / owning the rollover.
Rationale
We are using datastreams and ILM predefined on our elastic cluster and do not want micrometer to manage / own that portion. Since elastic offers size and time based index rollover - this (in my opinion) is something better left to the cluster and not the micrometer library.
Additional context
#2058
I think pulling more functionality into micrometer is adding to the complexity of the library and maybe not the right direction. Also, it may not stay compatible as versions of ealsticsearch change with the internal representations of ILM.
I can create a PR if helpfull - but would ask for guidance on backwards compatibly requirements / concerns.
The text was updated successfully, but these errors were encountered: