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

Improve promtail configuration docs #1602

Merged
merged 5 commits into from
Feb 5, 2020
Merged

Conversation

slim-bean
Copy link
Collaborator

Fixes #1593 (or at least improves)

The promtail configs were missing the docker and cri stages as well as had potentially confusing names in the docs vs the actual stage names.

Also the docker example was difficult to follow because every OS logs docker logs to a different location.

Signed-off-by: Edward Welch [email protected]

Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up! The configuration page definitely needs the same attention that the individual stage pages got. I just have a few minor nits about proper nouns and some other small feedback, but this is looking good.

docs/clients/promtail/configuration.md Outdated Show resolved Hide resolved
docs/clients/promtail/configuration.md Show resolved Hide resolved
docs/clients/promtail/configuration.md Show resolved Hide resolved
docs/clients/promtail/configuration.md Show resolved Hide resolved
docs/clients/promtail/configuration.md Outdated Show resolved Hide resolved
docs/clients/promtail/configuration.md Outdated Show resolved Hide resolved
docs/clients/promtail/configuration.md Outdated Show resolved Hide resolved
docs/clients/promtail/configuration.md Outdated Show resolved Hide resolved
docs/clients/promtail/configuration.md Outdated Show resolved Hide resolved
docs/clients/promtail/configuration.md Outdated Show resolved Hide resolved
```
① This location needs to be writeable by promtail.
② A `job` label is fairly standard in prometheus and useful for linking metrics and logs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we say which label is mandatory and why or is it already somewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

are any labels mandatory?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

well aside from __path__

Copy link
Contributor

Choose a reason for hiding this comment

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

I think __host__ is

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

Lgtm

@codecov-io
Copy link

Codecov Report

Merging #1602 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1602   +/-   ##
=======================================
  Coverage   60.63%   60.63%           
=======================================
  Files         106      106           
  Lines        8052     8052           
=======================================
  Hits         4882     4882           
  Misses       2784     2784           
  Partials      386      386

slim-bean and others added 5 commits February 5, 2020 14:27
…, added the docker and cri stages, clarified and improved the examples a little.

Signed-off-by: Edward Welch <[email protected]>
Signed-off-by: Edward Welch <[email protected]>
Signed-off-by: Edward Welch <[email protected]>
@slim-bean slim-bean merged commit cb79863 into master Feb 5, 2020
@slim-bean slim-bean deleted the improve-promtail-docs branch February 5, 2020 19:31
cyriltovena pushed a commit to cyriltovena/loki that referenced this pull request Jun 11, 2021
* added interfaces for store spceific limits

NewStore method in pkg/storage uses Overrides type defined in Cortex itself.
Added an interface to let consumers of that method pass any type which implements required methods
This is done as part of way to let projects using Cortex, define their own limits

Signed-off-by: Sandeep Sukhani <[email protected]>

* renamed interface CompositeStoreLimits to StoreLimits

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

Successfully merging this pull request may close these issues.

Docs: Promtail Docker config example seem out of date with the docs
4 participants