Set Time to MS

[martinkunc]

Set Time to MS to prevent server denial when measurement set doesn't contain any data except aggregation. For MSs with unset time and no other data being reported, I was getting server errors that time "Is too old"

[cce]

Hi @martinkunc the reporter is setting the Time on the whole batch later, after flushing these metrics, here: https://github.com/appoptics/appoptics-api-go/blob/master/reporter.go#L80 but I noticed your call is using Time.Now().UTC().Unix() and the code that's there is setting only Time.Now().Unix(). By any chance are you experiencing this error on hosts that are not using UTC as their local time zone? I think the right fix would be to change just the reporter code I linked to, rather than set the per-measurement time as in this PR.

(A long term fix would be to have the measurement aggregator flush using only per-measurement times rather than global times, this would make it easier to integrate with the BatchPersister and is #30)

[cce]

opened #32 as alternate fix, hopefully @martinkunc this would fix the error you're getting?

[martinkunc]

Chris, I tried to use Utc in the reporter (also measurement_batching and runtime), but it looks it is not helping. The message returned when I removed setting time for batch even with UTC. Actually The error message I am getting is:

time="2018-02-23T17:07:28+01:00" level=error msg="Error uploading AppOptics measurements batcherr{\"errors\":[{\"param\":\"time\",\"reason\":\"Is too old (>2 hours old). Check for local clock drift or enable NTP.\",\"value\":0}]}tryCount3abortingtrue" time="2018-02-23T17:07:42+01:00" level=info msg="error: {Errors:[map[param:time value:0 reason:Is too old (>2 hours old). Check for local clock drift or enable NTP.] map[param:sum reason:is required]]}\n"

The message is complaining about time being 0, so I think we need to set it also for batch. Do you have any other idea ? Maybe I missed some place where I should put .UTC ?

[martinkunc]

@cce Chris, do you have any other ideas. I tested setting Utc to reporter, but it didnt helped and error message is still returning zero in reported time. This should be still a working workaround..

[cce]

Hi @martinkunc sorry about this bug, figured it out while talking to @tillberg — the Time field that got added to the Measurement type in https://github.com/appoptics/appoptics-api-go/commit/e023f667426f414c6c28bc27fd12e47192407f35 doesn't contain an omitempty tag so it breaks the MeasurementSet reporter. Thanks for catching! Will fix and write a test to make sure it doesn't happen again ..

[trevrosen]

@martinkunc @cce derrrp sorry about that :-(

[cce]

No worries! Thanks to @tillberg for helping diagnose it ... fixed in #33!