Rename package name to `swift-tracing`

pokryfka commented 4 years ago

As the library and its interface matures, I think it would be great to change the package name to swift-tracing, even though the GIT repository name remains gsoc-swift-tracing.

ktoso commented 4 years ago

Right - once matured a bit and PoCed in a few real tracers we anticipate to pitch this project to the SSWG, rename and most likely move to swift-server or another organization which makes sense to indicate that this package is "the" standard. Renames would touch both the repo and module names.

The same for the baggage context repository.

I hope we could be trying to do this soon after the GSoC period.

pokryfka commented 4 years ago

My point is that, assuming you consider current API reasonably stable, the package name could be changed before repo change; In which case, after the repository is moved, the URL in Swift PM dependency would need to be update but not the package name would stay the same, example:

    dependencies: [
        // needs to be updated
        .package(url: "https://github.com/slashmo/gsoc-swift-tracing.git", .branch("main")),
    ],
    targets: [
        .target(
            name: "AnInstrument",
            dependencies: [
//                .product(name: "Instrumentation", package: "gsoc-swift-tracing"),
           // stays the same
                .product(name: "Instrumentation", package: "swift-tracing"),
            ]
        ),

ktoso commented 4 years ago

Sure, we could do that, but It's somewhat parallel to considering anything stable or not.

I'd suggest to be careful with assuming the only change would be just changing the url. It's true that even github handles redirects well in that case, and yea if the API is "done-ish" it'd be just a change or location. In reality the APIs could also be changing during SSWG review period as we uncover more things that need adjusting etc. So yeah we can do that but I don't think the benefit is as huge as one might imagine -- 1 line change vs 2 line change.

Do you have a specific reason in mind why this change would be very beneficial right now, other than not wanting the gsoc- prefix in the module just for visual reasons?

I don't mind changing right now btw, it's just that I don't think this changes much in reality :-)

ktoso commented 4 years ago

Summing up though: sure, we can change it now, I don't anticipate trouble originating from it.

pokryfka commented 4 years ago

Its pure marketing ;-)

For libraries/tools adding dependency on gsoc-swift-tracing "production" package name would:

slightly less work when URL changes (its true its all defined in the SPM manifest file so its hardly an effort)
indicates API is close to final version and so its "psychologically" easier to add dependency on it

then again I dont know how much the API is stable or is likely to be changed during review later on

ktoso commented 4 years ago

It's not stable at all but we can change the artifact name IF we know we'll move the repo somewhere else and this one will go away -- because otherwise it would end up pretty confusing.

I assume @slashmo is on board with this plan as we talked over it a few times, but to sanity check ccing here @slashmo. Happy to take a PR with the module name change I think.

ktoso commented 4 years ago

Note that the same applies to gsoc-swift-baggage-context

slashmo commented 4 years ago

@pokryfka Thanks for bringing this up 👍 I think we all agree that gsoc-swift-tracing definitely won't be the final name as it's just a WIP title. However, I think we should also consider the possibility of not including tracing in the name at all, as it kind of defeats the claim that it's a set of libraries for more than just tracing. Maybe it's a bit too brought, but what do you think about swift-instrumentation instead?

I'm also in favor of splitting up the Instrumentation library into multiple smaller libraries (#31). Then "Tracing" could be reflected in the library's name instead (e.g. TraceInstrumentation).

ktoso commented 4 years ago

Ah, thanks for the reminder -- I'd normally hammer on about this but I missed it this time 😅 True... We were considering putting the TracingInstrument in a package by itself... that could be swift-trace-instrumentation and the other being swift-baggage-instrumentation or perhaps swift-instrumentation indeed...

Food for thought before we jump on a name here I guess.

pokryfka commented 4 years ago

I think the naming and partitioning is up to you - @slashmo @ktoso

My argument is a bit more general: if the library is to be used outside its public interface/name needs to freeze (or start freezing ;-))

obviously the swift xray sdk is in "WIP alpha" stage itself, still I am a bit cautious about using packages and types which public interface is potentially very unstable

for instance, I see advantage of using the same type for BaggageContext when other libraries start using it as well or at the very least its name/tag/interface suggest its reasonably stable (which I dont think is the case at the moment)

ktoso commented 4 years ago

I don't think we're disagreeing :-)

Just we have not figured out the relative timing between projects yet, i.e.:

you'd you be ok to depend on your main branch on this main branch and we figure out as we go because you are in the middle of intense early development as well, OR
should we make a long-running branch for this purpose, OR
should we rename the package of baggage-context AND tag a version there since we anticipate that part of the puzzle to be pretty stable soon?

If we knew what we are optimizing for, in our collaboration, then we can pick a plan to follow :-)

Currently this project's "optimized for plan" is that it is a GSoC project and no one so far has been depending on it, and now we are in a phase where we need adoption to ensure the API matures in real use cases. I'm absolutely open to various approaches here but I don't think we can say right now that any of those APIs are getting frozen, though we can tag work in progress versions if that'd help.

E.g. with the baggage context I don't think it will be changing much, however - I do think we'll want to explore if making it a CoW type would be beneficial. In order to properly measure that, we need some real tracer using it, and compare in a real-ish sample the overheads introduced. (I do think the CoW will be useful, but I like to prove things, not just guess on them :-)).

Having that said, maybe it'd be best if we sync up (the 3 of us) in a short call soon to figure out how we want to approach the aligning of the APIs? You can shoot me an email at ktoso@apple.com and I'd schedule something quick, wdyt? :-)

--

for instance, I see advantage of using the same type for BaggageContext when other libraries start using it as well or at the very least its name/tag/interface suggest its reasonably stable (which I dont think is the case at the moment)

Yes exactly, that is going to be a huge benefit and the reason why the baggage context is separate and zero-dependencies. Because we'd want to avoid having to adapt between various "basically the same" container types of metadata.

If it'd help, perhaps we can remove the gsoc- from the package name there, and also tag it as 0.1, WDYT @pokryfka @slashmo ?

slashmo commented 4 years ago

If it'd help, perhaps we can remove the gsoc- from the package name there, and also tag it as 0.1

Sounds good to me. As you said BaggageContexts internals might change, but I guess the public API already is fairly stable.

pokryfka commented 4 years ago

If it'd help, perhaps we can remove the gsoc- from the package name there, and also tag it as 0.1

please do :-)

I want to finish the first version of XRayInstrument without adding dependency on BaggageContexts to the "core" XRayRecorder lib.

This means some bridging inside of XRayInstrument.

Then see how to add BaggageContexts to XRayRecorder itself. Note that for a user of XRayInstrument this will be a transparent "internal" change not affecting public API.

Now, as much I want to make XRayRecorder a good citizen in "swift-server" ecosystem, using BaggageContexts adds value ONLY if other libs use it as well (its a bit of a chicken-egg problem).

For a started it would be great to have it:

AsyncHTTPClient per https://github.com/slashmo/gsoc-swift-tracing/issues/46
NIO channel per https://github.com/slashmo/gsoc-swift-tracing/issues/48

if you have some time I would be happy to import updated versions from a develop repo/branch and test it all together with tracer (tracing instrument). I can also make PRs with changes addressing the above and have it discussed.

ktoso commented 4 years ago

The 2 step plan sounds good. Looking forward to more baggage adoption; we're certain that other SSWG projects will endorse it, just a matter of timing 👍

We'll ping once things are tagged on baggage.

Moritz will take on the slimming down of the APIs here :)

The chicken-and-egg will solve itself soon enough hopefully.

ktoso commented 4 years ago

As for the actual topic of the issue -- we'll do some renames shortly :)

ktoso commented 4 years ago

baggage 🧳 was renamed a little bit then.

this repo will follow soon

ktoso commented 3 years ago

I'm working on official repos, should land them soon.

slashmo / gsoc-swift-tracing

Rename package name to `swift-tracing` #78