singer-io / getting-started

This repository is a getting started guide to Singer.
https://singer.io
1.26k stars 148 forks source link

Middleware Specification (v0.1.0) Proposal #62

Closed pcorbel closed 4 years ago

pcorbel commented 5 years ago

A proposal on the implementation of a middleware type and LOG messages into the specification to make Singer more production ready

awm33 commented 5 years ago

@pcorbel It marked the one item outdated, so just posting it here instead.

Metrics are actually part of the spec, just not well documented https://github.com/singer-io/getting-started/blob/96a0f7addec517fcf5155284744c648fe4f16902/docs/SYNC_MODE.md#metric-messages

I think this is a great opportunity to make them easier to use, but I would elect to keep them separate in order to 1) Separate logging from monitor, related but separate roles in my mind 2) Keeping the current form will make it easier for existing users to adopt, so far everything is additive, this would be a breaking change.

pcorbel commented 5 years ago

@pcorbel It marked the one item outdated, so just posting it here instead.

Metrics are actually part of the spec, just not well documented https://github.com/singer-io/getting-started/blob/96a0f7addec517fcf5155284744c648fe4f16902/docs/SYNC_MODE.md#metric-messages

I think this is a great opportunity to make them easier to use, but I would elect to keep them separate in order to 1) Separate logging from monitor, related but separate roles in my mind 2) Keeping the current form will make it easier for existing users to adopt, so far everything is additive, this would be a breaking change.

Good find @awm33, I never noticed this documentation on metrics ;)

About your point 1), the point of the LOG type message is to be able to contains everything not related to the schema, state and records. Maybe the LOG name isn't a good one, and we need something more generic like CONTEXT or INFO...

About your point 2), I agree that it looks like a breaking change, however the documentation speak about Consumers of the tap logs which is something completely external of the Singer specification scope. So, from my point of view, it looks like a breaking change but it isn't one