Skip to content
This repository has been archived by the owner on Jan 19, 2024. It is now read-only.

docs(datadog-service): fix newline spacing #146

Merged

Conversation

vadasambar
Copy link
Contributor

@vadasambar vadasambar commented Jul 19, 2022

Signed-off-by: Suraj Banakar [email protected]
(cherry picked from commit 0821fbe)

Why?

Many lines in installation instructions show up as one line.
image
https://artifacthub.io/packages/keptn/keptn-integrations/datadog-service

This PR attempts to fix that. This PR only changes the spacing. No change in content was made.

Signed-off-by: Suraj Banakar <[email protected]>
(cherry picked from commit 0821fbe)
@github-actions

This comment has been minimized.

@vadasambar vadasambar marked this pull request as ready for review July 19, 2022 05:23
@vadasambar vadasambar requested a review from a team as a code owner July 19, 2022 05:23
Copy link
Member

@TannerGabriel TannerGabriel left a comment

Choose a reason for hiding this comment

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

Thank you for the update. I left a small comment.

export DD_API_KEY="<your-datadog-api-key>" DD_APP_KEY="<your-datadog-app-key>" DD_SITE="datadoghq.com"
helm install -n keptn datadog-service https://github.com/keptn-sandbox/datadog-service/releases/download/0.15.0/datadog-service-0.15.0.tar.gz --set datadogservice.ddApikey=${DD_API_KEY} --set datadogservice.ddAppKey=${DD_APP_KEY} --set datadogservice.ddSite=${DD_SITE}
```
export DD_API_KEY="<your-datadog-api-key>" DD_APP_KEY="<your-datadog-app-key>" DD_SITE="datadoghq.com"
Copy link
Member

Choose a reason for hiding this comment

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

It seems like we are repeating the following line quite often in the README file. Maybe we can add it as a prerequisite to prevent code duplication (not related to your current changes).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, we are using it at many places in the README. I will give it a thought (create a separate PR maybe?). Thank you @TannerGabriel. That being said, that's unrelated to this PR like you said. Can I get a stamp on this one? 🙏

Copy link
Contributor Author

@vadasambar vadasambar Jul 21, 2022

Choose a reason for hiding this comment

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

Created an issue: keptn-sandbox/datadog-service#41. Thank you for the feedback 🙏 If you notice something like this, I'd love to hear about it. You can just create an issue at https://github.com/keptn-sandbox/datadog-service/issues even if it's not an issue and just feedback or something you want to talk about around datadog-service. You can also reach out to me on Keptn slack at @vadasambar

Copy link
Contributor

@christian-kreuzberger-dtx christian-kreuzberger-dtx left a comment

Choose a reason for hiding this comment

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

Hi @vadasambar , I just checked, and this seems to be a common problem across our instal instructions :(

However, dynatrace-service seems to do it right:

install: "# Installation\n\nThe dynatrace-service can be installed in three steps:\n\
\n## 1. Download the latest dynatrace-service Helm chart\n\nDownload [the latest\
\ dynatrace-service Helm chart](https://github.com/keptn-contrib/dynatrace-service/releases/latest/)\
\ from GitHub. Please ensure that the version of the dynatrace-service is compatible\
\ with the version of Keptn you have installed by consulting the [Compatibility\
\ Matrix](compatibility.md). Details on installing or upgrading Keptn can be found\
\ on the [Keptn website](https://keptn.sh/docs/quickstart/).\n\n## 2. Gather Keptn\
\ credentials\n\nThe dynatrace-service requires access to the Keptn API consisting\
\ of `KEPTN_API_URL`, `KEPTN_API_TOKEN` and optionally `KEPTN_BRIDGE_URL`.\n\n*\
\ To get the values for `KEPTN_API_URL` (also known as `KEPTN_ENDPOINT`), please\
\ see [Authenticate Keptn CLI](https://keptn.sh/docs/0.16.x/operate/install/#authenticate-keptn-cli).\n\
\n* By default, the `KEPTN_API_TOKEN` is read from the `keptn-api-token` secret\
\ (i.e., the secret from the control-plane) and does not need to be set during installation.\n\
\n* If you would like to use backlinks from your Dynatrace tenant to the Keptn Bridge,\
\ provide the service with `KEPTN_BRIDGE_URL`. For further details about this value,\
\ please see [Authenticate Keptn Bridge](https://keptn.sh/docs/0.16.x/operate/install/#authenticate-keptn-bridge).\n\
\nIf running on a Linux or Unix based system, you can assign these to environment\
\ variables to simplify the installation process: \n\n```console\nKEPTN_API_URL=<KEPTN_API_URL>\n\
KEPTN_BRIDGE_URL=<KEPTN_BRIDGE_URL> # optional\n```\n\nAlternatively, replace the\
\ variables with the actual values in the `helm upgrade` command in the following\
\ section.\n\n\n## 3. Install the dynatrace-service\n\nTo install the dynatrace-service\
\ in the standard `keptn` namespace, execute:\n\n```console\nhelm upgrade --install\
\ dynatrace-service -n keptn \\\n <HELM_CHART_FILENAME> \\\n --set dynatraceService.config.keptnApiUrl=$KEPTN_API_URL\
\ \\\n --set dynatraceService.config.keptnBridgeUrl=$KEPTN_BRIDGE_URL\n```\n\n\
**Note:** \n- You can select additional installation options by appending key-value\
\ pairs with the syntax `--set key=value`.\n"

image
can you adapt your PR accordingly?

@vadasambar
Copy link
Contributor Author

Hi @vadasambar , I just checked, and this seems to be a common problem across our instal instructions :(

However, dynatrace-service seems to do it right:

install: "# Installation\n\nThe dynatrace-service can be installed in three steps:\n\
\n## 1. Download the latest dynatrace-service Helm chart\n\nDownload [the latest\
\ dynatrace-service Helm chart](https://github.com/keptn-contrib/dynatrace-service/releases/latest/)\
\ from GitHub. Please ensure that the version of the dynatrace-service is compatible\
\ with the version of Keptn you have installed by consulting the [Compatibility\
\ Matrix](compatibility.md). Details on installing or upgrading Keptn can be found\
\ on the [Keptn website](https://keptn.sh/docs/quickstart/).\n\n## 2. Gather Keptn\
\ credentials\n\nThe dynatrace-service requires access to the Keptn API consisting\
\ of `KEPTN_API_URL`, `KEPTN_API_TOKEN` and optionally `KEPTN_BRIDGE_URL`.\n\n*\
\ To get the values for `KEPTN_API_URL` (also known as `KEPTN_ENDPOINT`), please\
\ see [Authenticate Keptn CLI](https://keptn.sh/docs/0.16.x/operate/install/#authenticate-keptn-cli).\n\
\n* By default, the `KEPTN_API_TOKEN` is read from the `keptn-api-token` secret\
\ (i.e., the secret from the control-plane) and does not need to be set during installation.\n\
\n* If you would like to use backlinks from your Dynatrace tenant to the Keptn Bridge,\
\ provide the service with `KEPTN_BRIDGE_URL`. For further details about this value,\
\ please see [Authenticate Keptn Bridge](https://keptn.sh/docs/0.16.x/operate/install/#authenticate-keptn-bridge).\n\
\nIf running on a Linux or Unix based system, you can assign these to environment\
\ variables to simplify the installation process: \n\n```console\nKEPTN_API_URL=<KEPTN_API_URL>\n\
KEPTN_BRIDGE_URL=<KEPTN_BRIDGE_URL> # optional\n```\n\nAlternatively, replace the\
\ variables with the actual values in the `helm upgrade` command in the following\
\ section.\n\n\n## 3. Install the dynatrace-service\n\nTo install the dynatrace-service\
\ in the standard `keptn` namespace, execute:\n\n```console\nhelm upgrade --install\
\ dynatrace-service -n keptn \\\n <HELM_CHART_FILENAME> \\\n --set dynatraceService.config.keptnApiUrl=$KEPTN_API_URL\
\ \\\n --set dynatraceService.config.keptnBridgeUrl=$KEPTN_BRIDGE_URL\n```\n\n\
**Note:** \n- You can select additional installation options by appending key-value\
\ pairs with the syntax `--set key=value`.\n"

image
can you adapt your PR accordingly?

Will do. Thank you for looking that up for me. 🙇

@github-actions

This comment has been minimized.

- add `observability` and `monitoring` tags

Signed-off-by: Suraj Banakar <[email protected]>
@github-actions
Copy link
Contributor


✓ datadog-service 0.15.0 (datadog-service/0.15.0)

Package lint SUCCEEDED!

✓ Name: datadog-service
✓ Display name: datadog-service
✓ Version: 0.15.0
! App version: *** NOT PROVIDED ***
✓ Description: Keptn SLI provider for datadog
✓ License: Apache-2.0
! Logo URL: *** NOT PROVIDED ***
✓ Home URL: https://keptn.sh/docs/integrations/
✓ Deprecated: false
✓ Pre-release: false
✓ Contains security updates: false
! Provider: *** NOT PROVIDED ***
! Readme: *** NOT PROVIDED ***
✓ Keywords:

  • keptn
  • sandbox
  • observability
  • monitoring
    ✓ Links:
  • Name: Source | URL: https://github.com/keptn-sandbox/datadog-service/tree/release-0.15.0
    ! Maintainers: *** NOT PROVIDED ***
    ! Containers images: *** NOT PROVIDED ***
    ! Changes: *** NOT PROVIDED ***
    ! Recommendations: *** NOT PROVIDED ***
    ! Screenshots: *** NOT PROVIDED ***
    ✓ Operator: false
    ✓ Install: PROVIDED

1 package(s) found, 0 package(s) with errors

@vadasambar
Copy link
Contributor Author

@christian-kreuzberger-dtx can I get a review again. I have made the requested changes and one small change

Copy link
Contributor

@christian-kreuzberger-dtx christian-kreuzberger-dtx left a comment

Choose a reason for hiding this comment

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

LGTM

@christian-kreuzberger-dtx christian-kreuzberger-dtx merged commit f580836 into keptn-contrib:main Aug 12, 2022
afzal442 pushed a commit to Cloud-Hacks/artifacthub that referenced this pull request Oct 13, 2022
* docs(datadog-service): fix newline spacing

Signed-off-by: Suraj Banakar <[email protected]>
(cherry picked from commit 0821fbe)

* docs(datadog-service): use `\n` for newline spacing
- add `observability` and `monitoring` tags

Signed-off-by: Suraj Banakar <[email protected]>

Signed-off-by: Suraj Banakar <[email protected]>
Signed-off-by: afzal442 <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants