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

bump dependencies #227

Merged
merged 7 commits into from
Oct 28, 2020
Merged

bump dependencies #227

merged 7 commits into from
Oct 28, 2020

Conversation

rfratto
Copy link
Member

@rfratto rfratto commented Oct 27, 2020

All of these bumps depend on each other so it unfortunately has to be done in one massive PR.

  • Prometheus: bump to release-2.21-grafana
  • OTel: bump to 0.13.0
  • Loki: bump to 2.0

Closes #203.

I'm also sneaking in a bug fix for #228, where I updated all the docs for Tempo to refer to push_config but the code still expected remote_write for the Tempo fields.

Also Closes #228.

Requesting review from @joe-elliott for the pkg/tempo changes and @grafana/cloud-middleware for the broad changes.

@rfratto rfratto requested review from joe-elliott and a team October 27, 2020 17:50
@CLAassistant
Copy link

CLAassistant commented Oct 27, 2020

CLA assistant check
All committers have signed the CLA.

@rfratto
Copy link
Member Author

rfratto commented Oct 27, 2020

I've spent some time testing this e2e locally and everything works (scraping service, metrics, logs, traces). There's probably a good opportunity to add Tempo to the docker-compose and k3d examples in line with how we run Cortex and Loki, but that's out of scope of this PR.

@rfratto
Copy link
Member Author

rfratto commented Oct 27, 2020

This needs the docs to be updated to mention the new SD configs, but that can happen in parallel with a review (edit: this is done).

Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

Two comments. The links definitely need to be updated. After this is built let me know. I want to confirm that everything still works with Grafana Cloud.

pkg/tempo/config.go Outdated Show resolved Hide resolved
pkg/tempo/config.go Outdated Show resolved Hide resolved
- Replace queue with sending_queue and retry_on_failure
- Change default value for retry_on_failure.max_elapsed_time to 60s
- Update URLs to point to vendored version
- Update configs to use new config changes (including k8s manifest)
Copy link
Member

@joe-elliott joe-elliott 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 the otel updates! lgtm

@@ -41,11 +41,12 @@ type Config struct {

// RWConfig controls the configuration of exporting to Grafana Cloud
type RWConfig struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason you didn't change the name of this struct to go with the use of PushConfig above? I can't see the struct in use anywhere else.

@@ -88,13 +90,13 @@ service:
`,
},
{
name: "remote_write options",
name: "push_config options",
Copy link
Contributor

Choose a reason for hiding this comment

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

IDK whats going on with indent here, wasn't caused by your changes of course.

@rfratto rfratto merged commit 8df75ba into grafana:master Oct 28, 2020
@rfratto rfratto deleted the bump-deps branch October 28, 2020 18:47
mattdurham pushed a commit that referenced this pull request Nov 11, 2021
* bump dependencies

- otel: bump to 0.13.0
- prometheus: bump to release-2.21-grafana
- loki: bump to 2.0

* fix issue where tempo config was expecting remote_write over push_config

* update CHANGELOG

* fix tests I broke :)

* address review feedback

- Replace queue with sending_queue and retry_on_failure
- Change default value for retry_on_failure.max_elapsed_time to 60s
- Update URLs to point to vendored version
- Update configs to use new config changes (including k8s manifest)

* reference new prometheus service discovery mechanisms

* address feedback
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label May 2, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

push_config field not working in Tempo config Update to Prometheus v2.21.0
4 participants