feat(core): use nanosecond precision by default in Point.toLineProtocol()

[sranka]

Fixes #337

Proposed Changes

Briefly describe your proposed changes:

Checklist

• CHANGELOG.md updated
• Rebased/mergeable
• A test has been added if appropriate
• yarn test completes successfully
• Commit messages are in semantic format

[codecov-commenter]

Codecov Report

Merging #338 (18973b6) into master (b7c6c6b) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #338   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           32        32           
  Lines         1132      1144   +12     
  Branches       266       271    +5     
=========================================
+ Hits          1132      1144   +12     

Impacted Files	Coverage Δ
packages/core/src/Point.ts	100.00% <100.00%> (ø)
packages/core/src/util/currentTime.ts	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7c6c6b...18973b6. Read the comment docs.

[empz]

I'm confused by this line. The fact that the timestamp value is a number doesn't mean it's in NS, or does it?

Shouldn't this block be doing the following?

return Math.floor(value).toString().padEnd(19, '0')

This way it would ensure that no matter if the Point was constructed given a number in seconds or milliseconds, it'll allways pad it with 0s until reaching NS precision.

[sranka]

Passing a number never implied conversion to nanoseconds (even by default). A Date instance should be better used, it was/is a preferred choice since it contains an exact timestamp, it does not need further explanation. If you pass a number, its time unit (precision) is external to a point + you also cannot express precise nanoseconds with a number type. This all was the API contract before this PR, it was also documented that way.

Padding a number to 19 digits is not compliant with the previous behavior, it is also not something that I would like to introduce. Users can still use a string or a Date value for what they need, but a number value already has a different meaning to what you expect. Maybe that I am too deep in the js client, so that the documentation that seems clear to me is not clear enough :) I would appreciate your suggestions about changing the doc.

https://influxdata.github.io/influxdb-client-js/influxdb-client.point.timestamp.html :

Point.timestamp() method
Sets point timestamp. Timestamp can be specified as a Date (preferred), number, string or an undefined value. An
undefined value instructs to assign a local timestamp using the client’s clock. An empty string can be used to let the server
assign the timestamp. A number value represents time as a count of time units since epoch. The current time in
nanoseconds can’t precisely fit into a JS number, which can hold at most 2^53 integer number. Nanosecond precision
numbers are thus supplied as a (base-10) string. An application can use ES2020 BigInt to represent nanoseconds,
BigInt’s toString() returns the required high-precision string.

Note that InfluxDB requires the timestamp to fit into int64 data type.

[empz]

A number value represents time as a count of time units since epoch

That means I can do Point.timestamp(1620908660) (seconds) but also Point.timestamp(1620908660999) (milliseconds) ?

Your convertTimeToNanos function is supposed to return the timestamp in nanoseconds as a string, right?

So how does the following work?

const point = new Point("measurement").timestamp(1620908660); // seconds

console.log(point.toLineProtocol());

That falls into the block

else if (typeof value === 'number') {
    return String(Math.floor(value))
}

So the timestamp is simply converted to a string without adding the extra characters needed to be in nanosecond precision.

Or what am I missing here?

[empz]

...but a number value already has a different meaning to what you expect this is the part that is not clear, or at least it's not what I would expect.

If I say point.toLineProtocol() I expect to get the same output format no matter how the point was built. If this is not the case and making it so is not an option because it's a breaking change, I think the documentation for toLineProtocol() should make this very clear.

[sranka]

A number does not imply a nanosecond precision, it is a fact to accept independently what I or you think about. I am welcome to a documentation change for you to suggest.

[sranka]

I improved the doc in the Point class in the scope of this PR, I think that it is more explanatory now.

[empz]

/**
   * Creates an InfluxDB protocol line out of this instance.
   * @param settings - settings control serialization of a point timestamp and can also add default tags,
   * nanosecond timestamp precision is used when no settings are supplied.
   * @returns an InfluxDB protocol line out of this instance
   */
  public toLineProtocol(settings?: Partial<PointSettings>): string | undefined { }

I'm sorry to be a pain but I don't think this is an improvement at all. The added doc. comment says "nanosecond timestamp precision is used when no settings are supplied" which is not true.

Again, if I create a point like this:

const point = new Point("measurement").timestamp(1620908660); // seconds

Then do point.toLineProtocol(), without settings. The new doc states that I'll get nanosecond precision, which I won't because of this block:

else if (typeof value === 'number') {
    return String(Math.floor(value))
}

But either I'm crazy or this is really confusing. It would be nice to have a third opinion... @vlastahajek ?

[empz]

I think the confusion comes from the fact that toLineProtocol is a public method and you're implying it will always be used with the writeApi from this library.
My use case was different, I was simply creating points, serializing them with toLineProtocol() and sent them to a message broker for Telegraf to consume and write to InfluxDB 2.0, which failed because the timestamp wasn't in nanoseconds.
So it's kind of an inconsistency between this library, Telegraf and its influxdb_v2 output plugin.

Having said this, I still think the "toLineProtocol" method should use the same precision in the output no matter how the point was built in the first place.

For now, I'll just create my points with nanosecond strings for my use case.

[sranka]

I know that telegraf requires nanos and I know your UC since the beginning, I was quite recently sending line protocol data to telegraf through kafka. A developer that uses a number timestamp has to follow the improved documentation of Point.timestamp(). point.toLineProtocol() indeed uses nanosecond precision without settings, it really means that the supplied number is in nanoseconds. For example: 999999999999 number is

• 1970-01-01T00:16:39.999999999Z with nanosecond precision
• 2001-09-09T01:46:39.999Z with millisecond precision

The Point implementation is correct in the expectation now, 999999999999 cannot be 2286-11-20T17:46:39.999Z as you suggest. A number is quite problematic to absorb, that's why I already recommended to use Date instances.