elastic / elasticsearch-metrics-reporter-java

Metrics reporter, which reports to elasticsearch
Apache License 2.0
231 stars 103 forks source link

Daily index date is based on local date rather than UTC #52

Open garyhodgson opened 7 years ago

garyhodgson commented 7 years ago

Hi,

Because the reporter assumes that the date used in the index is the local date, rather than UTC (which ES presumes) we find that our metrics are being split over two indices (we are storing metrics in daily indices and are in CET (GMT+1) Timezone).

From the code I see that the reporter derives the index name from a Date object, and the SimpleDateFormat uses the local Timezone by default.
This means that at midnight Jan 25 CET the reporter send metrics to a new Jan 25 index, whereas the metrics are still timestamped Jan 24 UTC.

Setting the Timezone of the SimpleDateFormat resolves the problem. The following snippet hopefully helps to explain.

  long timestamp = 1485298800; // Wed Jan 25 00:00:00 CET 2017 / Wed Jan 24 23:00:00 UTC 2017

  System.out.println("timestamp = "+timestamp*1000);
  // timestamp = 1485298800000

  System.out.println("Date = " + new Date(timestamp*1000));
  // Date = Wed Jan 25 00:00:00 CET 2017

  String currentIndexName = "metrics";
  SimpleDateFormat indexDateFormat = new SimpleDateFormat("yyyy.MM.dd");
  currentIndexName += "-" + indexDateFormat.format(new Date(timestamp * 1000));
  System.out.println("currentIndexName (CET) = " + currentIndexName);
  // currentIndexName (CET) = metrics-2017.01.25

  currentIndexName = "metrics";
  indexDateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
  currentIndexName += "-" + indexDateFormat.format(new Date(timestamp * 1000));
  System.out.println("currentIndexName (UTC) = " + currentIndexName);
  // currentIndexName (UTC) = metrics-2017.01.24

To clarify why this is a problem for us (besides the ES indexes being UTC oriented), we run an aggregator on each index and then can simply delete the entire index. As it stands, an hours worth of values from the next day are missing, and are then bundled into the subsequent run. Furthermore, the logic in this reporter does not match how logstash allocates documents to indices, i.e. 00:00->23:59 UTC in one index.

Changing the behaviour of the reporter now might not be desirable, but adding the option to choose between UTC index dates or local would be very useful, or at least mentioning it in the documentation might help, in case it trips someone else up.

I'll happily create a Pull Request if you wish.

greg-pendlebury commented 6 years ago

I just noticed this bug as well. It is quite pronounced for us because Canberra is UTC + 11. I came in to work this morning (1st of Nov) and couldn't find any errors to indicate why all of the analytics systems showed no data, but it is because so far this morning all of the metrics are still recorded with a @timestamp of 31st of Oct in UTC. However the reporter is writing them into the 2018-11 index because it is choosing the index name based on local time instead of UTC time.

The 'bug' is that it should align the timezones with the indexes they are put into, instead of writing October timestamps into the November index.