sosreport / sos

A unified tool for collecting system logs and other debug information
http://sos.rtfd.org
GNU General Public License v2.0
508 stars 542 forks source link

[Feedback requested] Dropping redirectors and updating text output for sosreport #3374

Open TurboTurtle opened 1 year ago

TurboTurtle commented 1 year ago

It's been a few years since we released sos-4.0 and changed the binary to sos, and along with it clarified that the tool is sos, of which report is a function.

I'd like to propose dropping the sosreport and sos-collector redirectors in an upcoming release. I'd ideally say 4.7, but that's probably not feasible since we're in 4.6 currently and most downstreams, I'd assume, want a little more time to coordinate the change.

So how do we feel about 4.7 or 4.8 for no longer carrying these wrappers?

Secondly, along with this change I'd like to update the text output from sosreport to sos (space) report globally. We've got a most of the project using this for a bit now, but the former still in use in the initial banner text, as well as the most-likely-critical "Your sosreport has been generated" message. Note I am not proposing that the tarball name change in any way.

I imagine there is a decent amount of automation looking for that string (in fact, we ourselves look for it during sos collect runs), so this is a change that needs to be well communicated.

I'd like both of these changes to go out at the same time, so if we need to wait until 4.8 for our downstreams to prepare for it, then so be it.

pmoravec commented 1 year ago

Cc @jcastill and @mhradile at least.

AFAIK original plans were to keep the shortcut/redirectors for the sos-4.* timeframe. While the planned change itself is sound, my gut feeling is users are not yet prepared, still thinking about "sosreport (tarball) is taken by sosreport (command)". We do have a warning text Please note the 'sosreport' command has been deprecated but it is hard to say how many people read the leading text (or realize its implication). People usually run sosreport, wait for the outcome and copy the generated tarball. So maybe we shall put such string (also) at the end of the stdout where it gets more attention as a heads up, now?

Also, what would be the justification for the redirectors drop - just "we have the legacy redirectors for a while"? Isn't better message "here we add another sos subcommand (e.g. sos upload) under the sos umbrella and it is the good time to drop the redirectors"?

mhradile commented 1 year ago

+1 for keeping them. Our (old and nowhere near migrated) tests are definitely not ready.

On Mon, Oct 9, 2023 at 8:25 AM Pavel Moravec @.***> wrote:

Cc @jcastill https://github.com/jcastill and @mhradile https://github.com/mhradile at least.

AFAIK original plans were to keep the shortcut/redirectors for the sos-4.* timeframe. While the planned change itself is sound, my gut feeling is users are not yet prepared, still thinking about "sosreport (tarball) is taken by sosreport (command)". We do have a warning text Please note the 'sosreport' command has been deprecated but it is hard to say how many people read the leading text (or realize its implication). People usually run sosreport, wait for the outcome and copy the generated tarball. So maybe we shall put such string (also) at the end of the stdout where it gets more attention as a heads up, now?

Also, what would be the justification for the redirectors drop - just "we have the legacy redirectors for a while"? Isn't better message "here we add another sos subcommand (e.g. sos upload) under the sos umbrella and it is the good time to drop the redirectors"?

— Reply to this email directly, view it on GitHub https://github.com/sosreport/sos/issues/3374#issuecomment-1752408620, or unsubscribe https://github.com/notifications/unsubscribe-auth/AZTDIAW2UYLHCSOXBSGY2CTX6OKGRAVCNFSM6AAAAAA5WL7D6OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJSGQYDQNRSGA . You are receiving this because you were mentioned.Message ID: @.***>

TurboTurtle commented 1 year ago

AFAIK original plans were to keep the shortcut/redirectors for the sos-4.* timeframe.

I mean, we only went to 4.x instead of continuing the 3.x line because of the new binary. I don't see anything on the horizon that would want us to move to a 5.x versioning scheme (also, just going to sidestep the entire versioning scheme debate minefield here). Software evolves and we should be mindful of backwards compatibility for sure, but we shouldn't be shackled to maintaining old entrypoints.

While the planned change itself is sound, my gut feeling is users are not yet prepared, still thinking about "sosreport (tarball) is taken by sosreport (command)".

It's been over 3 years at this point; how much preparation is realistically needed?

Beyond that we should want to encourage an understanding that sos does more than "just" the report function. One of the easiest ways to do this is to have users notice "hey, report is a subcommand, what else can this thing do?"

We do have a warning text Please note the 'sosreport' command has been deprecated but it is hard to say how many people read the leading text (or realize its implication). People usually run sosreport, wait for the outcome and copy the generated tarball. So maybe we shall put such string (also) at the end of the stdout where it gets more attention as a heads up, now?

I'd definitely be open to adding a footer message for a minor version cycle before we drop the redirectors.

Also, what would be the justification for the redirectors drop - just "we have the legacy redirectors for a while"? Isn't better message "here we add another sos subcommand (e.g. sos upload) under the sos umbrella and it is the good time to drop the redirectors"?

As I recall, sos upload was shot down when I brought it up previously in an SST meeting - but less pedantically to your point of tying it to another subcommand being added...it'd be nice but I don't think we should marry the two. By that same token, if there are subcommands you/anyone think would be a useful addition let's get a discussion going on that separately from this one.

Our (old and nowhere near migrated) tests are definitely not ready.

I'm going to be politically nice here and not comment on this point.

jcastill commented 1 year ago

What about adding the text "Please note the 'sosreport' command has been deprecated" in 4.7, leave it for 4.8 (and perhaps expand it with a 'will be completely dropped in the next minor release', and drop redirectors in 4.9 (or even 4.10 if we consider that there will be 4.xx) ?

We have this documented in many places already, and two minor releases may give us enough time to finish whatever else is needed regarding communication.

Our (old and nowhere near migrated) tests are definitely not ready.

@mhradile we can discuss this, but hopefully two minor releases may give us enough time to plan for the change?

pmoravec commented 1 year ago

I am ok having that explicit statement "sosreport command will be removed in the (next | 4.X) release" around the "sosreport has been generated at" text, and react accordingly :)