Update loki's default port to 3100 #6349
Conversation
I was suprised this was not already the case. I think it would be a good change to make, but I am also contious of the breaking nature of this.
|
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
- querier/queryrange -0.1%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
There was a problem hiding this comment.
Not sure how I feel about this.
On the one hand, I like it (except for the fact that 3100 is already registered) and a lot of documentation refers to this port
But on the other hand I don't see much value in changing it at this stage
But on the other other hand we expose port 3100 in our docker image, so who the hell knows 🤷 🥲
I think this could cause trouble as you say, for relatively meagre gains.
|
IMO, everything that help simplifying the config is a win, of course mainly for new users (think adoption), since existing users already have the overrides in place. Regarding the Docker image: Yes, we |
There was a problem hiding this comment.
This is a good config simplification, I'm on board with it. @dannykopping are you thinking it's going to break a lot of existing installations because they're relying on this being port 80? I'm pretty sure we have this set in all our of deployments, as well as the jsonnet and helm libraries with publish.
Not overly worried, let's see what happens and mention it in our changelog/upgrade guide before merging pls @simonswine |
|
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
|
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0.3%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
|
@dannykopping take a look if the changelog entry works for you like this 👍 |
|
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
- distributor -0.3%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
Co-authored-by: Karen Miller <[email protected]>
|
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
osg-grafana
added
type/docs
I was suprised this was not already the case. I think it would be a good change to make, but I am also contious of the breaking nature of this.