feat: slightly configurable line writer #17396

hoorayimhelping · 2020-03-24T14:55:15Z

Adds a module that programmatically writes line data to the server, org and bucket specified.

here's a gist of it being used to implement RUM

CHANGELOG.md updated with a link to the PR (not the Issue)
Well-formatted commit messages
Rebased/mergeable
Tests pass

drdelambre · 2020-03-24T17:34:15Z

there's a bit more logic to this. try and inject the regexp filters from here to more closely follow the spec:
https://github.com/influxdata/github-issue-stats/blob/master/api/src/stat/line.js#L80

hoorayimhelping · 2020-03-24T20:27:55Z

@drdelambre

800a45d

asalem1 · 2020-03-24T21:12:52Z

ui/src/utils/lineWriter.ts

@@ -0,0 +1,129 @@
+import AJAX from 'src/utils/ajax'


Is there a reason we're using AJAX rather than fetch?

AJAX uses axios, which uses XMLHTTPRequest which is more widely supported

we polyfill fetch though right? since we are actively depreciating anything that uses axios (as it's currently only tied to the old api client), it would be cool to see new code not using it so that we can keep api auth mechanisms centralized in src/client (like where tokens are stored and header params and the like). Not a blocker, just bumps the work to the client library tech debt epic right off the bat.

@drdelambre my thinking is since this isn't a client interacting with our api, but making a request to an external service, it shouldn't really go in src/client. i was under the assumption that src/client is for communicating with our api via swagger. is that not the case? if it is, do we have a story around how to make requests to external services or resources? i assumed that was the intent of AJAX.

either way, first I'd heard we're actively deprecating anything that uses axios. an effective way to let people know this is to put a comment saying something is deprecated and shouldn't be used and link to a place that explains that. do we have any documentation like an rfc around deprecating axios that i can link to? or anything explaining what to use instead?

we do indeed polyfill fetch. would be nice not to rely on axios for this. this def doesn't need to (or should be) within src/client. i agree that we should just rip axois out altogether and eliminate the confusion.

We had a big talk about it during planning a month ago when bonitoo was releasing the new api client, and made these in an effort to put a bow on an effort that was 90% of the way there:
https://github.com/influxdata/influxdb/issues?q=is%3Aopen+is%3Aissue+label%3Ateam%2Fmonitoring+label%3Akind%2Ftech-debt+generated+client
Everyone has been busy doing other stuff though. We currently treat our queries as an external resource (weird right?) within the app, which you can get a gander of over here:
https://github.com/influxdata/influxdb/blob/master/ui/src/shared/apis/query.ts#L55

desa

Really awesome stuff. Only suggestion would be to add some kind of buffering/batching so that we don't flood the system with a bunch of small writes.

desa · 2020-03-24T21:36:44Z

ui/src/utils/lineWriter.ts

+// Builds a basic line writer that will write a line to the url, org and bucket specified.
+// Returns a function named `writeLine` that utilizes closures to access the configuration
+// arguments passed in to `buildLineWriter`.
+export const buildLineWriter = function buildLineWriter(


One thing we might want to consider it building up a buffer of metrics instead of just writing a single line. Maybe something on the order of ~100 metrics before making the actual network request.

Common patterns I've seen for influx writers are to have batch size and interval be configurable options. For example write 100 metrics in a batch or every 10s, whichever is first.

@desa
right on thanks! i figured that would be a suggestion :)

how does batching writes affect precision, if at all?

for that matter, what kind of precision is appropriate here? is second precision too coarse? could second-level precision lead to a situation where if there are two writes in the same second, the later one overwrites the earlier one?

Typically each point in a batch shares a common timestamp precision. Second precision here is probably fine. I cant imagine we'd want to track these with sub second precision

could second-level precision lead to a situation where if there are two writes in the same second, the later one overwrites the earlier one?

Could happen, but I think if we're reporting with that level of frequency we're doing something wrong

121watts

There's some solid work here! A nit, a suggestion, and something that should probably change before merging.

121watts · 2020-03-25T21:23:07Z

ui/src/utils/lineWriter.ts

+
+  constructor(
+    url: string,
+    orgId: number | string,


Suggested change

orgId: number | string,

orgID: number | string,

nit here (i prefer Id) actually. sadly, the rest of our app uses ID

121watts · 2020-03-25T21:29:51Z

ui/src/utils/lineWriter.ts

+    Object.keys(tags)
+      // Sort keys for a little extra perf
+      // https://v2.docs.influxdata.com/v2.0/write-data/best-practices/optimize-writes/#sort-tags-by-key
+      .sort()


Suggested change

.sort()

.sort(((a, b) () => a.localeCompare(b))

JavaScripts sort array prototype sorts on UTF-16 code units values. it wont sort lexicographically (alphabetically).

121watts · 2020-03-25T21:32:53Z

ui/src/utils/lineWriter.ts

@@ -0,0 +1,129 @@
+import AJAX from 'src/utils/ajax'


we do indeed polyfill fetch. would be nice not to rely on axios for this. this def doesn't need to (or should be) within src/client. i agree that we should just rip axois out altogether and eliminate the confusion.

drdelambre · 2020-03-25T21:42:55Z

ui/src/utils/lineWriter.ts

+  }
+}
+
+// Builds a basic line writer that will write a line to the url, org and bucket specified.


whats all this?

drdelambre

i wish the batchedWrite function had a name that more clearly indicated that it should be the default write function, and it would be cool if both write functions were polymorphic, in that they'd normalize between the line format and object format automatically, as objects are easier to mutate in state (append a default list of tags to every request for example) so they'll most likely become the dominant interface, making it weird to always call createLine and then batchedWrite, but i dont have any pressing reason this shouldn't be out in the world as is.

hoorayimhelping force-pushed the bs_line_writer branch 3 times, most recently from d09aa16 to fd0e947 Compare March 24, 2020 17:23

hoorayimhelping requested review from a team, 121watts and desa and removed request for a team March 24, 2020 17:26

hoorayimhelping force-pushed the bs_line_writer branch 2 times, most recently from cd36fb3 to 800a45d Compare March 24, 2020 20:27

asalem1 reviewed Mar 24, 2020

View reviewed changes

desa reviewed Mar 24, 2020

View reviewed changes

hoorayimhelping force-pushed the bs_line_writer branch 6 times, most recently from be3e3f1 to 68b642d Compare March 25, 2020 21:13

hoorayimhelping requested review from desa and drdelambre March 25, 2020 21:34

121watts suggested changes Mar 25, 2020

View reviewed changes

drdelambre reviewed Mar 25, 2020

View reviewed changes

hoorayimhelping force-pushed the bs_line_writer branch 6 times, most recently from c991945 to 58e1be2 Compare March 26, 2020 13:34

hoorayimhelping requested a review from 121watts March 26, 2020 14:34

hoorayimhelping force-pushed the bs_line_writer branch from 58e1be2 to 564a750 Compare March 26, 2020 15:18

hoorayimhelping requested a review from drdelambre March 26, 2020 16:41

drdelambre approved these changes Mar 26, 2020

View reviewed changes

feat: configurable js linewriter

a8d9ac5

hoorayimhelping force-pushed the bs_line_writer branch from 564a750 to a8d9ac5 Compare March 26, 2020 20:41

121watts approved these changes Mar 26, 2020

View reviewed changes

hoorayimhelping merged commit a142d11 into master Mar 26, 2020

hoorayimhelping deleted the bs_line_writer branch March 26, 2020 21:12

hoorayimhelping mentioned this pull request Mar 26, 2020

feat(sampledata): Add demo data dropdown and membership request to buckets #17454

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: slightly configurable line writer #17396

feat: slightly configurable line writer #17396

hoorayimhelping commented Mar 24, 2020 •

edited

Loading

drdelambre commented Mar 24, 2020

hoorayimhelping commented Mar 24, 2020

asalem1 Mar 24, 2020

hoorayimhelping Mar 24, 2020

asalem1 Mar 24, 2020

drdelambre Mar 25, 2020

hoorayimhelping Mar 25, 2020

121watts Mar 25, 2020

drdelambre Mar 25, 2020

desa left a comment

desa Mar 24, 2020

hoorayimhelping Mar 24, 2020

desa Mar 25, 2020

desa Mar 25, 2020

121watts left a comment

121watts Mar 25, 2020

121watts Mar 25, 2020

121watts Mar 25, 2020

drdelambre Mar 25, 2020

drdelambre left a comment

feat: slightly configurable line writer #17396

feat: slightly configurable line writer #17396

Conversation

hoorayimhelping commented Mar 24, 2020 • edited Loading

drdelambre commented Mar 24, 2020

hoorayimhelping commented Mar 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

121watts left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drdelambre left a comment

Choose a reason for hiding this comment

hoorayimhelping commented Mar 24, 2020 •

edited

Loading