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

Fix compatibility with Kubernetes 1.22 #36

Merged
merged 9 commits into from
Apr 29, 2022
Merged

Fix compatibility with Kubernetes 1.22 #36

merged 9 commits into from
Apr 29, 2022

Conversation

pr00se
Copy link
Contributor

@pr00se pr00se commented Apr 15, 2022

fixes #35

k8s 1.22 removed the beta endpoints for some resources, which caused the demo application to fail to install

This PR:

  • adds k8s-libsonnet 1.22 (and removes 1.20 and 1.21)
  • updates jsonnet libs using jb update
  • updates main.jsonnet to use non-beta Ingress resources

Copy link
Contributor

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@achatterjee-grafana
Copy link
Contributor

achatterjee-grafana commented Apr 25, 2022

I have added a few comments.
To give some background information, the readme was re-written a few months back to confirm to Grafana IA and style guide. We should only make the necessary doc updates to reflect code changes or dependency changes. The readme does not need a full copy-edit.

@pr00se, If you let me know the new content you need to add to the readme, I can update the docs for you.

Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

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

Added partial doc review.

README.md Outdated
## Overview

The New Stack (TNS) is a simple three-tier demo application, fully instrumented with the 3 pillars of observability: metrics, logs, and traces. It offers an insight on what a modern observability stack looks like and experience what it's like to pivot among different types of observability data.
The New Stack (TNS) is a simple demo application, fully instrumented with the 3 pillars of observability: metrics, logs, and traces. It offers an insight into what a modern observability stack looks like, and what it's like to pivot among different types of observability data.
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
The New Stack (TNS) is a simple demo application, fully instrumented with the 3 pillars of observability: metrics, logs, and traces. It offers an insight into what a modern observability stack looks like, and what it's like to pivot among different types of observability data.
The New Stack (TNS) is a simple demo application, fully instrumented with the 3 pillars of observability: metrics, logs, and traces. It offers an insight into what a modern observability stack looks like and experience what it's like to pivot among different types of observability data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think of "and allows you to experience"? "Experience" by itself doesn't match with the first item in the sentence.

README.md Outdated

![TNS workflow](./img/TNS-Arc-Diagram-1.png)

The TNS app is an example three-tier web app built by Weaveworks. It consists of a data layer, application logic layer, and load-balancing layer. To learn more about it, see [How To Detect, Map and Monitor Docker Containers with Weave Scope from Weaveworks](https://thenewstack.io/how-to-detect-map-and-monitor-docker-containers-with-weave-scope-from-weaveworks/).

The instrumentation for the TNS app is as follows:

- Metrics: Each tier of the TNS app exposes metrics on /metrics endpoints, which are scraped by the Grafana Agent. Additionally, these metrics are additionally tagged with exemplar information. The Grafana Agent then writes these metrics to a Prometheus (with remote-read enabled) for storage. [While the Prometheus could scrape metrics from the TNS App directly, the demo is configured to make the Agent the central point through which metrics, logs, and traces are collected. The Prometheus can be substituted for any backend which accepts Prometheus remote write, such as Thanos or Cortex.]
- Logs: Each tier of the TNS app writes logs to standard output or standard error which is captured by Kubernetes, which are then collected by the Grafana Agent, which forwards them on to Loki for storage.
- Metrics: Each tier of the TNS app exposes metrics, tagged with [exemplar](https://grafana.com/docs/grafana/latest/basics/exemplars/) information, on `/metrics` endpoints. These are scraped by the Grafana Agent and then written to a Prometheus (with remote-read enabled) for storage. (While the Prometheus could scrape metrics from the TNS App directly, the demo is configured to make the Agent the central point through which metrics, logs, and traces are collected. The Prometheus can be substituted for any backend which accepts Prometheus remote write, such as Thanos or Cortex.)
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
- Metrics: Each tier of the TNS app exposes metrics, tagged with [exemplar](https://grafana.com/docs/grafana/latest/basics/exemplars/) information, on `/metrics` endpoints. These are scraped by the Grafana Agent and then written to a Prometheus (with remote-read enabled) for storage. (While the Prometheus could scrape metrics from the TNS App directly, the demo is configured to make the Agent the central point through which metrics, logs, and traces are collected. The Prometheus can be substituted for any backend which accepts Prometheus remote write, such as Thanos or Cortex.)
- Metrics: Each tier of the TNS app exposes metrics, tagged with [exemplar](https://grafana.com/docs/grafana/latest/basics/exemplars/) information, on `/metrics` endpoints. These are scraped by the Grafana Agent and then written to a Prometheus (with remote-read enabled) for storage. (While the Prometheus could scrape metrics from the TNS App directly, it is configured to collect metrics, logs, and traces through the Agent. You can substitute Prometheus with any backend which accepts Prometheus remote write, such as Thanos or Cortex.)

Copy link
Contributor Author

@pr00se pr00se Apr 25, 2022

Choose a reason for hiding this comment

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

The correction makes it seem the Prometheus is configured to collect metrics through the Agent, but it is the agent that is configured to collect the metrics and send them to Prometheus. What do you think of:

Suggested change
- Metrics: Each tier of the TNS app exposes metrics, tagged with [exemplar](https://grafana.com/docs/grafana/latest/basics/exemplars/) information, on `/metrics` endpoints. These are scraped by the Grafana Agent and then written to a Prometheus (with remote-read enabled) for storage. (While the Prometheus could scrape metrics from the TNS App directly, the demo is configured to make the Agent the central point through which metrics, logs, and traces are collected. The Prometheus can be substituted for any backend which accepts Prometheus remote write, such as Thanos or Cortex.)
- Metrics: Each tier of the TNS application exposes metrics, tagged with [exemplar](https://grafana.com/docs/grafana/latest/basics/exemplars/) information, on `/metrics` endpoints. These are scraped by the Grafana Agent and then written to a Prometheus (with remote-read enabled) for storage. While the Prometheus could scrape metrics from the TNS application directly, the Agent is configured to be the central point through which metrics, logs, and traces are collected. You can replace the Prometheus with any backend which accepts Prometheus remote write, such as Thanos, Cortex, or Mimir.

README.md Outdated Show resolved Hide resolved
README.md Outdated
@@ -1,193 +1,198 @@
# The New Stack (TNS) observability app

Copy link
Contributor

Choose a reason for hiding this comment

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

Pls, add the TOC here. It adds structure to the readme. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GitHub autogenerates a TOC from the markdown heading and keeps it in sync when things change. Do we prefer to keep a copy of the TOC in the body of the document as well? Looking at other examples of documentation (https://github.com/grafana/mimir, https://github.com/grafana/agent/tree/main/example/k3d) it doesn't seem common practice, but happy to add it back if it is preferred.

README.md Outdated

### K3D
**Note:** Ensure that your Docker daemon has a minimum of 2.5 GB of total memory available for all pods in this demo to be scheduled.
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
**Note:** Ensure that your Docker daemon has a minimum of 2.5 GB of total memory available for all pods in this demo to be scheduled.
**Note:** Ensure that your Docker daemon has a minimum of 2.5 GB of total memory available for all pods in this demo application.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why the preference away from "scheduled"? Pod scheduling is what would be impacted by insufficient memory, so it seems prudent to mention here (it was also part of the original copy).

README.md Outdated

### Tanka
If you would like to run the TNS demo using Docker Compose rather than Kubernetes, see instructions [here](/production/docker-compose/README.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Too many sentences starting with "If you would like to". We should keep the earlier bulleted list here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the "If you would like" in ce7c172

README.md Outdated

The following instructions will help you go from metrics to logs to traces.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why were these changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The instructions themselves? As someone new to the ecosystem working through the demo for the first time, I found some of the instructions lacked the clarity necessary to easily navigate the demo. Some of them were also outdated (e.g.: 'select "Prometheus" as the datasource' should instead be "prometheus-exemplars" in order to see exemplar information). Are there specific changes you'd like made?

README.md Outdated

This is likely because the jaeger agent is not running correctly. Check that all pods were successfully scheduled.

### `permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock` error when creating `k3d` cluster
Copy link
Contributor

Choose a reason for hiding this comment

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

The heading should be reworded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to "Permission error when creating k3d cluster" in ce7c172

@pr00se
Copy link
Contributor Author

pr00se commented Apr 25, 2022

@achatterjee-grafana replied to comments and tried to address the requested changes. I tried to update the instructions for clarity based on my experience installing/running/working through the demo. If the preference is to leave things as they are and simply update what has changed in this PR, I can revert the README changes and limit the scope of the PR to just the k8s 1.22 changes. Also happy to pull the README changes into a separate PR if that's easier -- let me know your preference! :)

Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

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

Added minor copy -edit suggestions, otherwise LGTM

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
@pr00se pr00se merged commit 10673b5 into main Apr 29, 2022
@pr00se pr00se deleted the k8s-1.22 branch April 29, 2022 16:02
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.

Kubernetes 1.22 compatibility
3 participants