Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate Tracer procedures #3442

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rh-max
Copy link
Contributor

@rh-max rh-max commented Nov 8, 2024

What changes are you introducing?

Consolidating two procedures on configuring Tracer, which have a lot of overlap.

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

https://issues.redhat.com/browse/SAT-28625

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

Checklists

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

  • Foreman 3.13/Katello 4.15 (Satellite 6.17)
  • Foreman 3.12/Katello 4.14 (Satellite 6.16)
  • Foreman 3.11/Katello 4.13 (orcharhino 6.11 on EL8 only)
  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • Foreman 3.6/Katello 4.8
  • Foreman 3.5/Katello 4.7 (Satellite 6.13; orcharhino 6.6/6.7)
  • We do not accept PRs for Foreman older than 3.5.

@rh-max rh-max added Needs tech review Requires a review from the technical perspective and removed Not yet reviewed labels Nov 8, 2024
Copy link

github-actions bot commented Nov 8, 2024

@@ -1,18 +1,31 @@
[id="enabling-tracer-on-a-host_{context}"]
= Enabling Tracer on a host
[id="Configuring_Tracer_on_a_host_{context}"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[id="Configuring_Tracer_on_a_host_{context}"]
[id="configuring-tracer-on-a-host"]

If we already change it, we could drop "context" too which we've not decided but done in the past regularly.

Very much option/not blocking your PR.


Use this procedure to enable Tracer on {Project} and access Traces.
Tracer displays a list of services and applications that need to be restarted.
You can install Tracer on {ProjectName} and access Traces.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
You can install Tracer on {ProjectName} and access Traces.
You can install Tracer on hosts and access Traces on {Project}.

. In the {ProjectWebUI}, navigate to *Hosts* > *All Hosts*.
. Click the name of the host you want to modify.
. On the *Traces* tab, click *Enable Traces*.
. Select the provider to install `katello-host-tools-tracer` from the list.
+
Alternatively, install Tracer using the CLI:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really want to document this?

If so, it'd probably just be the same commands as the template. Also, "yum" only works for EL7+.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can, but then we should use the proper heading for a CLI procedure:

Suggested change
Alternatively, install Tracer using the CLI:
[id="cli-Configuring..."]
.CLI procedure

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But with ".CLI procedure", we generally refer to Foreman/Smart Proxies, not commands on hosts. I like that there's abstraction for the commands on hosts through the REX job templates.

@@ -24,7 +24,10 @@ include::modules/proc_changing-the-environment-of-a-host.adoc[leveloffset=+1]

include::modules/proc_changing-the-managed-status-of-a-host.adoc[leveloffset=+1]

include::modules/proc_enabling-tracer-on-a-host.adoc[leveloffset=+1]
:parent-context: {context}
:context: administering-hosts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any particular reason to change the context for this procedure?
Unless it's absolutely necessary to change the context, I would avoid it. Generally, it prevents problems with broken links in the web UI.

. In the {ProjectWebUI}, navigate to *Hosts* > *All Hosts*.
. Click the name of the host you want to modify.
. On the *Traces* tab, click *Enable Traces*.
. Select the provider to install `katello-host-tools-tracer` from the list.
+
Alternatively, install Tracer using the CLI:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can, but then we should use the proper heading for a CLI procedure:

Suggested change
Alternatively, install Tracer using the CLI:
[id="cli-Configuring..."]
.CLI procedure

Comment on lines 25 to 26
. Click *Enable Tracer*.
You get a REX job notification after the remote execution job is complete.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step looks like it doesn't belong to the CLI procedure. It should probably go above, together with the web UI procedure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to have would be to find out on what operating systems Tracer can be installed if there are any apart from RHEL and add this info to the introduction of the procedure.

@pr-processor pr-processor bot added the Waiting on contributor Requires an action from the author label Nov 14, 2024
Copy link
Contributor

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am currently working on the same issue in downstream docs and will provide a patch/some insight on Tracer on different Client OS.

. In the {ProjectWebUI}, navigate to *Hosts* > *All Hosts*.
. Click the name of the host you want to modify.
. On the *Traces* tab, click *Enable Traces*.
. Select the provider to install `katello-host-tools-tracer` from the list.
+
Alternatively, install Tracer using the CLI:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But with ".CLI procedure", we generally refer to Foreman/Smart Proxies, not commands on hosts. I like that there's abstraction for the commands on hosts through the REX job templates.

. Click *Enable Tracer*.
You get a REX job notification after the remote execution job is complete.
. Upload the Tracer data to the {ProjectName} server:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
. Upload the Tracer data to the {ProjectName} server:
. Upload the Tracer data to your {ProjectServer}:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs tech review Requires a review from the technical perspective Waiting on contributor Requires an action from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants