docs: fix getting started guide #512
Conversation
This commit fixes a series of issues in the getting started guide: - Avoid using relative paths for docker volumes. Use `pwd` instead: moby/moby#4830 (comment) - Fix indentation in prometheus config - Fix otcollector example - Missing span processor - Remove sampling_initial and sampling_thereafter keys in config
mauriciovasquezbernal
force-pushed
the
mauricio/fix-getting-started-guide
branch
from
0006bf2
to
8b3af86
Compare
Codecov Report
@@ Coverage Diff @@
## master #512 +/- ##
==========================================
- Coverage 89.48% 89.34% -0.14%
==========================================
Files 43 43
Lines 2215 2215
Branches 250 250
==========================================
- Hits 1982 1979 -3
- Misses 161 162 +1
- Partials 72 74 +2
Continue to review full report at Codecov.
|
docs/getting-started.rst
Outdated
|
|
||
| And start a docker container for it: | ||
|
|
||
| .. code-block:: sh | ||
|
|
||
| # --net=host will not work properly outside of Linux. | ||
| docker run --net=host -v ./prometheus.yml:/etc/prometheus/prometheus.yml prom/prometheus\ | ||
| docker run --net=host -v `pwd`/prometheus.yml:/etc/prometheus/prometheus.yml prom/prometheus \ |
There was a problem hiding this comment.
thoughts on maybe just calling out /tmp/ as a directory here? I've found that little tricks like backticks and the pwd command get lost on people newer to linux or programming in general.
There was a problem hiding this comment.
You're right, another issue is that pwd is unix only. I updated it to /tmp/.
| @@ -433,6 +431,7 @@ And execute the following script: | |||
| ) | |||
| tracer_provider = TracerProvider() | |||
| trace.set_tracer_provider(tracer_provider) | |||
| span_processor = BatchExportSpanProcessor(span_exporter) | |||
There was a problem hiding this comment.
An aside, but should we be using SimpleSpanProcessors in our examples? I feel like being available as an example is the point.
There was a problem hiding this comment.
That's a good question. I personally prefer to use the batch one because I don't want to create the impression that the simple is the default choice. Maybe we could adopt the convention of using SimpleSpanProcessor for the console span exporter and the BatchSpanProcessor for the others.
mauriciovasquezbernal
added
the
needs reviewers
mauriciovasquezbernal
added
the
PR:please merge
This commit fixes a series of issues in the getting started guide:
pwdinstead:docker -v /host:/container relative paths (volume relative path) moby/moby#4830 (comment)
To reviewers:
Please check that the old configuration isn't actually working for you and the new one solves it.