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

Clarification and plugin-inclusion #12

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

joergdw
Copy link
Contributor

@joergdw joergdw commented Jan 30, 2025

This PR adds two points:

  • It is clarified that custom scaling by a second app is only supported when “Basic Authentication“ is not used.
  • It introduces some content about the cf-cli-plugin for Autoscaler.

@joergdw joergdw changed the title Mention that scaling by other app is only supported with mTLS; Clarify requirements for custom scaling by second app Jan 30, 2025
@joergdw joergdw changed the title Clarify requirements for custom scaling by second app Clarification and plugin-inclusion Jan 30, 2025
@joergdw joergdw marked this pull request as ready for review January 31, 2025 15:02
“Application Autoscaler” offers a plug-in for the “Cloud Foundry CLI”. With it users can attach or detach scaling-policies to their apps. They can view them and view the scaling-history. And they can view the metrics send by their apps.

This is useful to get some insights on how currently applied scaling-policies work and if changes are reasonable. It is as well useful for debugging. A common case is, when [custom-metrics](<../defining-a-custom-metric-87e657e.md>) are introduced to scale an app and the developers want to check, if the send metrics are correctly processed by Autoscaler and why up-/down-scaling happens or does not happen.

Copy link

Choose a reason for hiding this comment

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

It's very helpful to mention the autosclaer cf cli plugin in the documentation! that's definitely an improvement. from my point of view, we shouldn't repeat ourself by copying the complete content of https://github.com/cloudfoundry/app-autoscaler-cli-plugin/blob/main/README.md into the help.sap.scom-documentation one more time. from my point of view, linking to it is enough 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My impression is, that our content was intended to be self-contained, so that you don't need to leave our help-page. The other approach would be to just transfer the good content of our help-page to the repository “app-autoscaler-release”. Then we consume everything from there. Using GitHub's “permalinks” makes it possible for us, to still be consistent and history-safe when we change the upstream-repository.

@silvestre and perhaps @sapsy, what do you think?

@@ -0,0 +1,210 @@
# Plug-in for the “Cloud Foundry CLI”
“Application Autoscaler” offers a plug-in for the “Cloud Foundry CLI”. With it users can attach or detach scaling-policies to their apps. They can view them and view the scaling-history. And they can view the metrics send by their apps.
Copy link
Contributor

Choose a reason for hiding this comment

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

Also thank you for using the correct name "Application Autoscaler" here!

I think further down there's quite a few mentions of "AutoScaler". Can we replace them with the correct (for SAP) name, "Application Autoscaler" as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sometimes it is what the cli responds, e.g.:

$ cf asa 'https://autoscaler.cf.stagingaws.hanavlab.ondemand.com'
Setting AutoScaler api endpoint to https://autoscaler.cf.stagingaws.hanavlab.ondemand.com...
OK

In those cases I would not diverge from the output of the cli. In all other I'll fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, we can as well change now that output of the cli. What do you think? @silvestre

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Btw. I have set a non-breaking whitespace in Application Autoscaler for better readability because then both words will be enforced to stay at the same line. (Personally I prefer the more visible Application-Autoscaler which also shows to humans that it belongs together, but I don't want to touch that here.)

@joergdw joergdw force-pushed the clarification_and_plugin branch from 12b17ba to c0eb6f6 Compare February 13, 2025 13:07
@joergdw joergdw requested a review from silvestre February 13, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants