axiom-js icon indicating copy to clipboard operation
axiom-js copied to clipboard

Option to flatten ingestion data

Open prokopec-simon opened this issue 2 years ago • 8 comments

Option fixing a minor issue I came across and reported as #46

The main goal is allowing users to flatten data for logging if needed, mostly for preventing circular references in objects resulting in a Type Error when trying to JSON.stringify such data. Examples of naturally circular structures are Node's request object or some exception objects.

I've added it as an Ingestion option rather than resolving to it by default. The main reasons for that are that in many cases I would prefer to get an exception when trying to log circular data and figuring out why that's happening, the second one possibly being performance. I couldn't find any serious enough resources comparing the performance of default JSON.stringify and flatted stringify, although I reckon there shouldn't be much difference.

I decided to use a package for the flattening rather than trying to achieve it myself, even though it seems doable in just a few lines of code. I reckon that the added dependency and the tiny increase in size (0.5K) are worth it.

prokopec-simon avatar Aug 02 '23 16:08 prokopec-simon

The build step is failing, but it doesn't seem related to your changes, I reran the build step again, lets see how it goes.

thesollyz avatar Aug 15 '23 14:08 thesollyz

I checked the other PRs, build step is fine, can you double check the build error?

thesollyz avatar Aug 16 '23 10:08 thesollyz

I don't have a v18 node installed, it seems like on 19.x it passes and I use 20.x and it passes too. I'll get v18 at some point and try to run the tests again.

prokopec-simon avatar Aug 16 '23 10:08 prokopec-simon

I tried running it with 18.17 Node and all tests are passing just fine too. Not sure where to look for the issue. I'll look at the CI/CD Pipelines later, but it's not my strength. Below is the output of Node 18.17 passing pnpm test


PS C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js> node -v v18.17.1 PS C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js> pnpm test

[email protected] test C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js turbo run test

╭────────────────────────────────────────────────────────────────────────╮ │ │ │ Update available v1.10.3 ≫ v1.10.12 │ │ Changelog: https://github.com/vercel/turbo/releases/tag/v1.10.12 │ │ Run "npx @turbo/codemod update" to update │ │ │ │ Follow @turborepo for updates: https://twitter.com/turborepo │ ╰────────────────────────────────────────────────────────────────────────╯ WARNING failed to contact turbod. Continuing in standalone mode: connection to turbo daemon process failed. To quickly resolve the issue, try running: - $ turbo daemon clean

    To debug further - please ensure the following:
    - the process identified by the pid in the file at C:\Users\PROKOP~1.SIM\AppData\Local\Temp\turbod\7ba40347da0ef5e4\turbod.pid is not running, and remove C:\Users\PROKOP~1.SIM\AppData\Local\Temp\turbod\7ba40347da0ef5e4\turbod.pid
    - check the logs at C:\Users\prokopec.simon\AppData\Roaming\turborepo\data\logs\7ba40347da0ef5e4-axiom-js.log
    - the unix domain socket at C:\Users\PROKOP~1.SIM\AppData\Local\Temp\turbod\7ba40347da0ef5e4\turbod.sock has been removed

    You can also run without the daemon process by passing --no-daemon

• Packages in scope: @axiomhq/js, @axiomhq/pino, @axiomhq/winston, examples-js, examples-pino, examples-winston, integration-tests, nextjs-app • Running test in 8 packages • Remote caching disabled @axiomhq/js:test: cache hit, replaying logs 55d11efb0df31c57 @axiomhq/js:test: @axiomhq/js:test: > @axiomhq/[email protected] test C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js\packages\js @axiomhq/js:test: > jest test/unit @axiomhq/js:test: @axiomhq/js:test: PASS test/unit/batch.test.ts @axiomhq/js:test: PASS test/unit/users.test.ts @axiomhq/js:test: PASS test/unit/datasets.test.ts @axiomhq/js:test: PASS test/unit/browser.test.ts @axiomhq/js:test: PASS test/unit/client.test.ts (7.809 s) @axiomhq/js:test: A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks. Active timers can also cause this, ensure that .unref() was called on them. @axiomhq/js:test: @axiomhq/js:test: Test Suites: 5 passed, 5 total @axiomhq/js:test: Tests: 21 passed, 21 total @axiomhq/js:test: Snapshots: 0 total @axiomhq/js:test: Time: 9.098 s @axiomhq/js:test: Ran all test suites matching /test\unit/i. @axiomhq/js:build:esm: cache hit, replaying logs b4752e67437471e8 @axiomhq/js:build:esm: @axiomhq/js:build:esm: > @axiomhq/[email protected] build:esm C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js\packages\js @axiomhq/js:build:esm: > tsc --project tsconfig.esm.json @axiomhq/js:build:esm: @axiomhq/winston:build:esm: cache hit, replaying logs 49510af8a61acd06 @axiomhq/js:build: cache hit, replaying logs 1eaa169bbddc347d @axiomhq/pino:build:esm: cache hit, replaying logs adc7a65c9955df2a @axiomhq/winston:build:esm: @axiomhq/winston:build:esm: > @axiomhq/[email protected] build:esm C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js\packages\winston @axiomhq/winston:build:esm: > tsc --project tsconfig.esm.json @axiomhq/winston:build:esm: @axiomhq/js:build: @axiomhq/js:build: > @axiomhq/[email protected] build C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js\packages\js @axiomhq/js:build: > tsc -b @axiomhq/pino:build:esm: @axiomhq/pino:build:esm: > @axiomhq/[email protected] build:esm C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js\packages\pino @axiomhq/pino:build:esm: > tsc --project tsconfig.esm.json @axiomhq/js:build: @axiomhq/pino:build:esm: @axiomhq/winston:test: cache hit, replaying logs 646c134b0bf7b604 @axiomhq/pino:test: cache hit, replaying logs 7d37dd5246ab2ce5 @axiomhq/pino:test: @axiomhq/pino:test: > @axiomhq/[email protected] test C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js\packages\pino @axiomhq/pino:test: > jest test/unit @axiomhq/pino:test: @axiomhq/pino:test: PASS test/unit/create-transport.spec.ts @axiomhq/pino:test: pino transport tests @axiomhq/pino:test: √ creates a truthy instance (6 ms) @axiomhq/winston:test: @axiomhq/pino:test: @axiomhq/pino:test: Test Suites: 1 passed, 1 total @axiomhq/winston:test: > @axiomhq/[email protected] test C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js\packages\winston @axiomhq/winston:test: > jest test/unit @axiomhq/pino:test: Tests: 1 passed, 1 total @axiomhq/pino:test: Snapshots: 0 total @axiomhq/winston:test: @axiomhq/pino:build: cache hit, replaying logs dee31d794cdff4f8 @axiomhq/winston:build: cache hit, replaying logs d87287bd5e81f9c7 @axiomhq/pino:test: Time: 4.4 s @axiomhq/pino:test: Ran all test suites matching /test\unit/i. @axiomhq/winston:test: PASS test/unit/create-transport.spec.ts @axiomhq/winston:test: winston transport tests @axiomhq/winston:test: √ creates a truthy instance (52 ms) @axiomhq/winston:test: @axiomhq/winston:test: Test Suites: 1 passed, 1 total @axiomhq/pino:build: @axiomhq/winston:build: @axiomhq/winston:test: Tests: 1 passed, 1 total @axiomhq/winston:test: Snapshots: 0 total @axiomhq/winston:test: Time: 4.456 s @axiomhq/winston:test: Ran all test suites matching /test\unit/i. @axiomhq/pino:build: > @axiomhq/[email protected] build C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js\packages\pino @axiomhq/winston:build: > @axiomhq/[email protected] build C:\Users\prokopec.simon\source\repos\prokopec-simon\axiom-js\packages\winston @axiomhq/winston:build: > tsc -b @axiomhq/winston:build: @axiomhq/pino:build: > tsc -b @axiomhq/pino:build:

Tasks: 9 successful, 9 total Cached: 9 cached, 9 total Time: 503ms >>> FULL TURBO

prokopec-simon avatar Aug 16 '23 13:08 prokopec-simon

I've tried to run the tests locally, one of them fails, something about fetch-retry. the main branch builds fine. not sure whats causing this yet. here is the error:

Jest has detected the following 1 open handle potentially keeping Jest from exiting:
@axiomhq/js:test:
@axiomhq/js:test:   ●  TCPWRAP
@axiomhq/js:test:
@axiomhq/js:test:       20 |     const headers = { ...this.config.headers, ...init.headers };
@axiomhq/js:test:       21 |
@axiomhq/js:test:     > 22 |     const resp = await fetchRetry(fetch)(finalUrl, {
@axiomhq/js:test:          |                                         ^
@axiomhq/js:test:       23 |       retries: 3,
@axiomhq/js:test:       24 |       retryDelay: function (attempt, error, response) {
@axiomhq/js:test:       25 |         return Math.pow(2, attempt) * 1000; // 1000, 2000, 4000
@axiomhq/js:test:
@axiomhq/js:test:       at ../../node_modules/.pnpm/[email protected]/node_modules/jest-mock/build/index.js:709:23
@axiomhq/js:test:       at wrappedFetch (../../node_modules/.pnpm/[email protected]/node_modules/fetch-retry/index.js:71:9)
@axiomhq/js:test:       at ../../node_modules/.pnpm/[email protected]/node_modules/fetch-retry/index.js:131:7
@axiomhq/js:test:       at fetchRetry (../../node_modules/.pnpm/[email protected]/node_modules/fetch-retry/index.js:63:12)
@axiomhq/js:test:       at FetchClient.<anonymous> (src/fetchClient.ts:22:41)
@axiomhq/js:test:       at step (src/fetchClient.ts:59:23)
@axiomhq/js:test:       at Object.next (src/fetchClient.ts:40:53)
@axiomhq/js:test:       at src/fetchClient.ts:34:71
@axiomhq/js:test:       at Object.<anonymous>.__awaiter (src/fetchClient.ts:30:12)
@axiomhq/js:test:       at FetchClient.Object.<anonymous>.FetchClient.doReq (src/fetchClient.ts:90:16)
@axiomhq/js:test:       at FetchClient.Object.<anonymous>.FetchClient.post (src/fetchClient.ts:50:17)
@axiomhq/js:test:       at AxiomWithoutBatching.BaseClient._this.ingestRaw (src/client.ts:43:17)
@axiomhq/js:test:       at AxiomWithoutBatching.Object.<anonymous>.AxiomWithoutBatching.ingest (src/client.ts:154:17)
@axiomhq/js:test:       at test/unit/client.test.ts:207:13
@axiomhq/js:test:       at Object.<anonymous> (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/toThrowMatchers.js:74:11)
@axiomhq/js:test:       at Object.throwingMatcher [as toThrow] (../../node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:312:21)
@axiomhq/js:test:       at test/unit/client.test.ts:208:12
@axiomhq/js:test:       at step (test/unit/client.test.ts:44:23)
@axiomhq/js:test:       at Object.next (test/unit/client.test.ts:25:53)
@axiomhq/js:test:       at test/unit/client.test.ts:19:71
@axiomhq/js:test:       at Object.<anonymous>.__awaiter (test/unit/client.test.ts:15:12)
@axiomhq/js:test:       at Object.<anonymous> (test/unit/client.test.ts:197:88)

thesollyz avatar Aug 18 '23 13:08 thesollyz

oh, just noticed, the AxiomWithoutBatching class has the flattedStringify inside of it, and that class fails. The other class with batching support didn't include flattedStringify, so we need to add it their as well.

thesollyz avatar Aug 18 '23 13:08 thesollyz

I think the issue has to do with the way you return the falttened data, and the syntax of async expect. for example here I think you should return

     const json = array
       .map((v) => {
-        options?.flattenData ? flattedStringify(v) : JSON.stringify(v);
+        return options?.flattenData ? flattedStringify(v) : JSON.stringify(v);
       })

and in the expect you are not actually returning the ingest method, and no awaiting for it:

expect(() => {
      axiom.ingest('test', [circularReferenceObject]);
}).toThrow(TypeError);

probably it would be await expect(axiom.ingest('test', [circularReferenceObject]); and maybe at the end it will have something like .rejects or something like that.

thesollyz avatar Aug 18 '23 14:08 thesollyz

Even though the changes should be minimal, I currently don't have the capacity to fix these problems. I'm using a fork that seems to work well, so it's probably just the tests. Once i have more free time I'll try to fix the issues. Thanks for your assistance.

prokopec-simon avatar Sep 06 '23 10:09 prokopec-simon