Skip to content
This repository was archived by the owner on Apr 23, 2021. It is now read-only.

Rename package name to swift-tracing #78

Closed
pokryfka opened this issue Jul 20, 2020 · 17 comments
Closed

Rename package name to swift-tracing #78

pokryfka opened this issue Jul 20, 2020 · 17 comments
Labels
Milestone

Comments

@pokryfka
Copy link
Contributor

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
Copy link
Collaborator

ktoso commented Jul 20, 2020

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
Copy link
Contributor Author

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
Copy link
Collaborator

ktoso commented Jul 20, 2020

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
Copy link
Collaborator

ktoso commented Jul 20, 2020

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

@pokryfka
Copy link
Contributor Author

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
Copy link
Collaborator

ktoso commented Jul 20, 2020

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
Copy link
Collaborator

ktoso commented Jul 20, 2020

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

@slashmo
Copy link
Owner

slashmo commented Jul 20, 2020

@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
Copy link
Collaborator

ktoso commented Jul 20, 2020

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
Copy link
Contributor Author

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
Copy link
Collaborator

ktoso commented Jul 20, 2020

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 [email protected] 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
Copy link
Owner

slashmo commented Jul 20, 2020

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
Copy link
Contributor Author

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:

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
Copy link
Collaborator

ktoso commented Jul 22, 2020

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
Copy link
Collaborator

ktoso commented Jul 22, 2020

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

@ktoso
Copy link
Collaborator

ktoso commented Jul 25, 2020

baggage 🧳 was renamed a little bit then.

this repo will follow soon

@ktoso ktoso changed the title Consider changing package name to swift-tracing Rename package name to swift-tracing Aug 7, 2020
@ktoso ktoso modified the milestones: Coding Period 3, 0.1.0 Sep 1, 2020
@ktoso
Copy link
Collaborator

ktoso commented Oct 9, 2020

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

@ktoso ktoso closed this as completed Oct 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants