fix(changetracking): alter the functioning of milli and nanoseconds in timestamps #1151
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
A bug has been reported recently via New Relic support, in which it has been stated that upon specifying timestamps which are NOT
time.Now()
with thechangetracking
package, such as the followinginstead of
seem to throw an error that states that the API has failed to validate the timestamp sent, as it is not within 24 hours of the current time.
Upon further investigation, the trend causing this error was identified: if the value of
nanoseconds
is 0, an error is thrown, and the error does not seem to be thrown with non zero values of nanoseconds, until the specified timestamp is out of 24 hours of the current time.This is because it appears that the API does not take timestamps with 0 milliseconds (as milliseconds cannot be a null value). Because of the absence of a field that can help specify milliseconds in Golang native types (
time.Time
), the implementation of the datatypenrtime.EpochMilliseconds()
relies on the value of nanoseconds to calculate milliseconds, which would be sent to the API. As a result, if nanoseconds is 0, milliseconds is subsequently 0 too, causing the error.In order to fix this error, the first approach adopted was #1137, proposing a fix to the intrinsic behaviour of the nrtime.EpochMilliseconds datatype to have non zero milliseconds when nanoseconds is zero; however, this seemed to break the functioning of a multitude of other functions which use this datatype.
To avoid such a breaking change, the other solution adopted (#1139) was to duplicate nrtime.EpochMilliseconds specific to the changetracking package (to have changetracking.EpochMilliseconds), which would then comprise the implementation already defined in #1137, so that the change would apply only to changetracking. However, it was later realised that this would amount to a breaking change (for those who are using the Go Client directly and invoking functions in the changetracking package).
As a result, a final workaround that has been adopted is the current PR, in which the value of milliseconds sent to the API is made non zero (when nanoseconds is zero), by adding a value to nanoseconds, when it is found to be zero. This has been added inside the function which calls the NerdGraph mutation, and is hence, vulnerable to removal upon
tutone generate
, but should not be discarded.Test cases have been added to validate the same.