datalust / seq-logging

A Node.js client for the Seq HTTP ingestion API
Apache License 2.0
25 stars 15 forks source link

works in node and browser #64

Closed u-rogel closed 1 year ago

u-rogel commented 1 year ago

This is only a draft, but works.

u-rogel commented 1 year ago

I needed to replace the required modules: http & https with the fetch api which is available out of the box in node v18. Since the imported module made no difference anymore there was no need for the url module since the protocol is less of a concern. Then next issue was the Buffer class. I saw Blob can achieve the same use case and since node v18 is available as well in node as well as in the browser.

KodrAus commented 1 year ago

Hi @u-rogel :wave:

Thanks for looking into this! I've just converted the PR to a draft for you while it's still in-progress. It looks like the only lingering question left is the retrying on timeouts?

u-rogel commented 1 year ago

Reply to @liammclennan from the issue #61 Unfortunately I am not aware of any such polyfill module. Only solution I am aware of is this one - from node-fetch. Also I don't know if that is really an upgrade for the node only module, hence might make sense to bump the version. Alternatively, would it be possible to also make a newer release for the sub-modules of winston-seq & pino-seq.

@KodrAus thanks for labelling it as draft. Yes, working out the timeouts retry is still missing. Will need to check if it can be done easily on both envs or do we need to search for some other solution.

u-rogel commented 1 year ago

Timeout handling added. So left with fetch polyfill for node < 18. Let me know what you guys think.

u-rogel commented 1 year ago

I think I found a workaround missing packages in the browser. Now it requires node-fetch@2 and abort-controller for the polyfills but works. I tested in the browser with an own react project, with your browser tool in the example folder and both worked. Also I tested with node 18 without polyfills and works too. Last but not least I tested with node 14 & node 16 after installing the two packages and the example script ran fine as well. I did left for you guys to do the proper adjustments for the package.json file and also if you want to add some to the README.md or so. Can you spot any open topic?

u-rogel commented 1 year ago

@liammclennan thanks! Do you know when I can expect it be available on npm?

liammclennan commented 1 year ago

@u-rogel Won't be this week. I'm still tidying up a few things, fixing the tests, and making sure it works everywhere.

As soon as that is done I will publish the package.

Thanks for this work, and for your patience through the process.

u-rogel commented 1 year ago

@liammclennan no rush, I will use my own branch till then. Thanks as well for the process on your end. It is my first PR to an open-source project, so a bit exciting for me.

Another mini contribution is for future development, would be great to add something like this to the example folder or something similar for spinning up a seq-server:

version: '3'
services:
  seq:
    image: datalust/seq:latest
    deploy: 
      resources:
        limits:
          memory: 14G
        reservations:
          memory: 14G
    # volumes:
    #   - /datadrive:/data
    environment:
      - ACCEPT_EULA=Y
    ports:
      - 80:80
      - 5341:5341
    expose:
      - 80
      - 5341
    labels:
      - traefik.backend=seq
      - traefik.frontend.rule=Host:seq.<snip>.com
      - traefik.port=80
liammclennan commented 1 year ago

Thanks @u-rogel

Something like that might be nice.