go-kit / kit

A standard library for microservices.
https://gokit.io
MIT License
26.51k stars 2.43k forks source link

Go kit 0.4.0 release #427

Closed peterbourgon closed 7 years ago

peterbourgon commented 7 years ago

Edit: Due to lots of feature churn in the last few weeks/months, I've been convinced that it makes more sense to hold off on a 1.0.0 until things settle. Notes in comments below.

Hello! I want to cut a v1.0.0 release in the near future. This issue is created per the request of @ChrisHines to serve as a notice to anyone who may be interested, for example to raise questions, concerns, or propose specific additions.

The only thing outstanding that I'd like to get in is a promotion of log/experimental_level to log/level. I've asked @seh to weigh in on that, as he's previously commented on the package. Otherwise, let me know your thoughts. I'll leave this open until at least the weekend, so another 5 days.

edit: outstanding issues I'd want to resolve before a 1.0.0

If anyone feels strongly about either of those two Optionals, (cc: @willfaught) please get your PRs in within the next few days...

ghost commented 7 years ago

Is some kind of connector architecture planned? If so, shouldn't this be part version 1?

On Jan 10, 2017 8:21 AM, "Peter Bourgon" notifications@github.com wrote:

Hello! I want to cut a v1.0.0 release in the near future. This issue is created per the request https://github.com/go-kit/kit/pull/368#issuecomment-271614515 of @ChrisHines https://github.com/ChrisHines to serve as a notice to anyone who may be interested, for example to raise questions, concerns, or propose specific additions.

The only thing outstanding that I'd like to get in is a promotion of log/experimental_level to log/level. I've asked @seh https://github.com/seh to weigh in on that, as he's previously commented on the package. Otherwise, let me know your thoughts. I'll leave this open until at least the weekend, so another 5 days.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/go-kit/kit/issues/427, or mute the thread https://github.com/notifications/unsubscribe-auth/ADGwwk9XGLP7iU41yHJZHFpOHjo6_oVmks5rQ7ACgaJpZM4Lfmpz .

peterbourgon commented 7 years ago

What is a connector architecture?

ChrisHines commented 7 years ago

What will be the compatibility policy after v1.0.0 has been released?

ghost commented 7 years ago

Connectors would be packages that implement the endpoint, transport, security, and other policies required by calling or called SaaS services like Salesforce.com, Workday, ServiceNow, LinkedIn, or Marketo. The connector architecture describes the wrapping of a SaaS service to call them or the wrapping of the go-kit service to be called by a SaaS services.

peterbourgon commented 7 years ago

@pruggera No, nothing like this is planned.

@ChrisHines I plan to follow typical SemVer rules. Do you think that is sufficient?

ChrisHines commented 7 years ago

I plan to follow typical SemVer rules. Do you think that is sufficient?

That's good. We shall see if it is sufficient. ;) Do we want to document this anywhere?

seh commented 7 years ago

Please see yet another leveled logging proposal in #444.

willfaught commented 7 years ago

Is there remaining work that needs to happen for the context or level tasks?

I have a couple questions about the experimental level package:

peterbourgon commented 7 years ago

Is there remaining work that needs to happen for the context or level tasks?

Yes, I need to get around to fixing #421, and I'm waiting for a response on #444.

Isn't it possible to unintentionally filter out loggings with keys that aren't actually coming from the level package? For example, mylogger.Log("error", err) where mylogger is from levels.New and is configured to ignore the error level. The person setting the log level doesn't necessarily have visibility into what keys code might be using for Log, and vice versa. Should the levels be typed with a string alias like for context keys to prevent conflicts?

Yes, but not with that example. You'd need to invoke it like logger.Log("level", "error", "oops", "this may be ignored"). But given that we've made the executive decision early on that levels are nothing more than key=val pairs, then I see logger.Log(..., "level", "error", ...) as a pretty explicit declaration of intent. With that said, I'm open to more strictly typing the level key, and introspecting on that type via reflection. I'd accept a PR :)

Why not use functional options for level.New instead of Config?

There's no practical difference, but I can in hindsight see the value of using functional options for consistency with the rest of package log. I'd accept a PR :)

peterbourgon commented 7 years ago

Update: 1.0.0 is just pending #470. ETA next week?

ChrisHines commented 7 years ago

I should be able to finish up #470 this week.

ChrisHines commented 7 years ago

470 is merged. We still need to bring #449 up to date and merge it.

ChrisHines commented 7 years ago

I've also added evaluating #474 to the check list.

basvanbeek commented 7 years ago

I will add ClientResponseFunc handling in the gRPC transport so we can pull in metadata sent back with a server response (think upstream baggage)

Also will add the ability to call BeforeFuncs/AfterFuncs multiple times to append in a Client request. (We already have done this for the Servers)

yurishkuro commented 7 years ago

I would like #403 and #463 addressed as they would change the discovery API. For us the metrics and discovery abstractions are the two primary reasons for using go-kit in Jaeger.

peterbourgon commented 7 years ago

We're already 2 months late on 1.0.0, changes to package sd will not be included. But I have no problem cutting a 1.1.0 or 2.0.0 very soon afterwards.

willfaught commented 7 years ago

Just curious, why is there a push to cut a v1? If I recall correctly, go-kit uses semver, and v0 is for unstable APIs during initial development. I don't understand the point of cutting a v1 if there will be an incompatible v2 shortly after. Why not keep it at v0 for now and wait until things have settled down?

On Sat, Mar 4, 2017 at 11:17 AM Peter Bourgon notifications@github.com wrote:

We're already 2 months late on 1.0.0, changes to package SD will not be included. But I have no problem cutting a 1.1.0 or 2.0.0 very soon afterwards.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/go-kit/kit/issues/427#issuecomment-284174244, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD5VrqYqpxhww7c3SM2roXI8nK7H5pdks5ribjagaJpZM4Lfmpz .

peterbourgon commented 7 years ago

Because I have made several intonations over the past several months that I would cut a 1.0.0 around the turn of the new year.

willfaught commented 7 years ago

OK. It seems to me the version should reflect the code, rather than the reverse, though.

On Sat, Mar 4, 2017 at 11:27 AM Peter Bourgon notifications@github.com wrote:

Because I have made several intonations over the past several months that I would cut a 1.0.0 around the turn of the new year.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/go-kit/kit/issues/427#issuecomment-284174901, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD5VvijXAWdM7TZ6RGz3YmGr_ISVZFuks5ribspgaJpZM4Lfmpz .

seh commented 7 years ago

I took the push for a version 1 to mean, "If you're thinking about changing any public interfaces, do it now and get it over with, because you won't get the chance to do so for a while afterward. Prioritize pending interface changes over other would-be contributions."

willfaught commented 7 years ago

Agreed; but ideas can lead to other ideas; who knows when that rabbit hole ends? I'm not saying doing v1 now and getting to v5 in six months is objectively wrong, but it seems burdensome to me for those depending on go-kit. At least with v0 you know there can/will be a lot of compatibility churn as things get hashed out.

The leveled logger was just added. Is a week or two long enough to say it's stable?

On Sat, Mar 4, 2017 at 11:39 AM Steven E. Harris notifications@github.com wrote:

I took the push for a version 1 to mean, "If you're thinking about changing any public interfaces, do it now and get it over with, because you won't get the chance to do so for a while afterward. Prioritize pending interface changes over other would-be contributions."

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/go-kit/kit/issues/427#issuecomment-284175656, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD5Vo4LeSCMYbr_wTcPUizdRmS0bXIjks5rib3_gaJpZM4Lfmpz .

seh commented 7 years ago

The leveled logger was just added. Is a week or two long enough to say it's stable?

As the author of three proposals (none of which made it in) for that leveled logger, having seen how much of the design space we explored and discarded, I do expect the interface is stable. There is room for further optimization, but the exported interface is the one that @ChrisHines and @peterbourgon want.

peterbourgon commented 7 years ago

After a night of reflection I agree that the feature churn is currently too high to justify a 1.0.0. I guess this is a good problem to have :)

So, I'll bundle up the current master + #474 into a 0.4.0 release. I want to wait for #474 because I think it will be a straightforward, boilerplate-reducing change. It will also bring API described in the logging proposal in line with what we have here.

Then, if @yurishkuro's work in package sd goes well, we can cut a 0.5.0 with that, as soon as it's stable. Letting that bake until mid-year and cutting a 1.0.0 for (say) GopherCon.

Thanks for the input, everyone.

ChrisHines commented 7 years ago

474 is merged.

We should consider updating the package docs for log/level to remove the experimental disclaimer before tagging.

peterbourgon commented 7 years ago

We should consider updating the package docs for log/level to remove the experimental disclaimer before tagging.

Oops, that was an oversight. Thanks for the spot. Fixed in master.

peterbourgon commented 7 years ago

OK, great. I'll cut 0.4.0 tomorrow.