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

Sidecar: failed to fetch initial external labels. unknown unit #4656

Closed
hanjm opened this issue Sep 12, 2021 · 7 comments · Fixed by #4657 or alecthomas/units#12
Closed

Sidecar: failed to fetch initial external labels. unknown unit #4656

hanjm opened this issue Sep 12, 2021 · 7 comments · Fixed by #4657 or alecthomas/units#12

Comments

@hanjm
Copy link
Member

hanjm commented Sep 12, 2021

Thanos, Prometheus and Golang version used:

Object Storage Provider:
COS
What happened:
thanos sidecar can not start up.
error message is

2021-09-12T16:09:13.532144772+08:00 level=warn ts=2021-09-12T08:09:13.531921453Z caller=sidecar.go:179 msg="failed to fetch initial external labels. Is Prometheus running? Retrying" err="parse Prometheus config: global:\n  scrape_interval: 30s\n  scrape_timeout: 10s\n  evaluation_interval: 1m\n  external_labels:\n    hash_mod: \"0\"\n    replica: A\n    tenant_id: default\nalerting:\n  alertmanagers:\n  - follow_redirects: true\n    scheme: http\n    timeout: 10s\n    api_version: v2\n    static_configs:\n    - targets: []\nrule_files:\n- /prometheus/recording_rule.yaml\n- /prometheus/alert_rule.yaml\nscrape_configs:\n- job_name: prometheus\n  honor_timestamps: true\n  scrape_interval: 10s\n  scrape_timeout: 10s\n  metrics_path: /metrics\n  scheme: http\n  follow_redirects: true\n  static_configs:\n  - targets:\n    - localhost:9090\n- job_name: etcd_sd\n  honor_timestamps: true\n  scrape_interval: 30s\n  scrape_timeout: 10s\n  metrics_path: /metrics\n  scheme: http\n  body_size_limit: 10485760\n  sample_limit: 15000\n  target_limit: 3000\n  label_name_length_limit: 255\n  label_value_length_limit: 512\n  follow_redirects: true\n: units: unknown unit  in 10485760"

prometheus new version instroduce a exempriment config field body_size_limit, the type is units. Base2Bytes (https://github.com/alecthomas/units). it implement text.TextUnmarshaler but not umplement text.TextMarshaler (https://github.com/alecthomas/units/blob/master/bytes.go#L43).
so the marshalled config content can not be marshalled.

What you expected to happen:

How to reproduce it (as minimally and precisely as possible):
Prometheus 2.29.2 + Thanos v0.23.0-rc.0

Full logs to relevant components:

Anything else we need to know:

@hanjm
Copy link
Member Author

hanjm commented Sep 12, 2021

It could be fixed by only unmarshal global config. I am fixing it.

@yeya24
Copy link
Contributor

yeya24 commented Sep 12, 2021

Let's add Prometheus 2.29.2 version to our E2E test so that we can catch the error in CI.

@hanjm
Copy link
Member Author

hanjm commented Sep 13, 2021

Let's add Prometheus 2.29.2 version to our E2E test so that we can catch the error in CI.

Yes, It will be great. but i see this TODO https://github.com/thanos-io/thanos/blob/main/test/e2e/e2ethanos/services.go#L44 // TODO(bwplotka): Run against multiple?, current E2E does not support multi version.

@yeya24
Copy link
Contributor

yeya24 commented Sep 13, 2021

Let's add Prometheus 2.29.2 version to our E2E test so that we can catch the error in CI.

Yes, It will be great. but i see this TODO https://github.com/thanos-io/thanos/blob/main/test/e2e/e2ethanos/services.go#L44 // TODO(bwplotka): Run against multiple?, current E2E does not support multi version.

OK. I see, it should be unit tests running by Circle CI. I think that supports different Prometheus versions.

As for E2E tests with multiple Prometheus versions, is it supported by the new E2E framework? cc @matej-g @bwplotka

@matej-g
Copy link
Collaborator

matej-g commented Sep 13, 2021

As for E2E tests with multiple Prometheus versions, is it supported by the new E2E framework? cc @matej-g @bwplotka

If I understand the question correctly, this should be already possible - have a look at signature of NewPrometheus, look at parameter promImage. Just specify the Docker image for whichever version you would like to run and instantiate the service.

@hanjm
Copy link
Member Author

hanjm commented Sep 13, 2021

If I understand the question correctly, this should be already possible - have a look at signature of NewPrometheus, look at parameter promImage. Just specify the Docker image for whichever version you would like to run and instantiate the service.

This way could only run single test which use specific promImage, not all the E2E test about sidecar.

@matej-g
Copy link
Collaborator

matej-g commented Sep 13, 2021

Do we want to run all the E2E tests against multiple versions of Prometheus? I guess then we could just pass environment variable with Prometheus version we desire and replace the image version (similarly to what we do for Thanos - https://github.com/matej-g/thanos/blob/main/test/e2e/e2ethanos/services.go#L54). I don't think there is any such feature 'baked' into the E2E framework.

hanjm added a commit to hanjm/thanos that referenced this issue Sep 14, 2021
hanjm added a commit to hanjm/thanos that referenced this issue Sep 15, 2021
yeya24 pushed a commit that referenced this issue Sep 16, 2021
…tes config type (#4657)

* Sidecar: Fix process external label when promethues v2.28+ use units.Bytes config type (#4656)

Signed-off-by: hanjm <[email protected]>

* E2E: Upgrade prometheus image version

Signed-off-by: hanjm <[email protected]>
bwplotka pushed a commit that referenced this issue Sep 20, 2021
…tes config type (#4657)

* Sidecar: Fix process external label when promethues v2.28+ use units.Bytes config type (#4656)

Signed-off-by: hanjm <[email protected]>

* E2E: Upgrade prometheus image version

Signed-off-by: hanjm <[email protected]>
kakkoyun pushed a commit that referenced this issue Sep 20, 2021
* Sidecar: Fix process external label on promethues v2.28+ use units.Bytes config type (#4657)

* Sidecar: Fix process external label when promethues v2.28+ use units.Bytes config type (#4656)

Signed-off-by: hanjm <[email protected]>

* E2E: Upgrade prometheus image version

Signed-off-by: hanjm <[email protected]>

* upgrade Prometheus dependency version to v2.30.0 (#4669)

* upgrade Prometheus dependency version to v2.30.0

Signed-off-by: Ben Ye <[email protected]>

* fix unit test

Signed-off-by: Ben Ye <[email protected]>
# Conflicts:
#	go.mod
#	go.sum

* Query: Fix (*exemplarsStream).receive/(*metricMetadataStream).receive/(*targetsStreamStream).receive infinite loop when target response Unimplemented error (#4676) (#4681)

Signed-off-by: hanjm <[email protected]>

* Cut 0.23.0-rc.1

Signed-off-by: Bartlomiej Plotka <[email protected]>

Co-authored-by: Jimmiehan <[email protected]>
Co-authored-by: Ben Ye <[email protected]>
bwplotka added a commit that referenced this issue Oct 6, 2021
* Cut release 0.23.0-rc.0 (#4625)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Updated version.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Cut 0.23.0-rc.1 and cherry picked 3 critical commits from main. (#4684)

* Sidecar: Fix process external label on promethues v2.28+ use units.Bytes config type (#4657)

* Sidecar: Fix process external label when promethues v2.28+ use units.Bytes config type (#4656)

Signed-off-by: hanjm <[email protected]>

* E2E: Upgrade prometheus image version

Signed-off-by: hanjm <[email protected]>

* upgrade Prometheus dependency version to v2.30.0 (#4669)

* upgrade Prometheus dependency version to v2.30.0

Signed-off-by: Ben Ye <[email protected]>

* fix unit test

Signed-off-by: Ben Ye <[email protected]>
# Conflicts:
#	go.mod
#	go.sum

* Query: Fix (*exemplarsStream).receive/(*metricMetadataStream).receive/(*targetsStreamStream).receive infinite loop when target response Unimplemented error (#4676) (#4681)

Signed-off-by: hanjm <[email protected]>

* Cut 0.23.0-rc.1

Signed-off-by: Bartlomiej Plotka <[email protected]>

Co-authored-by: Jimmiehan <[email protected]>
Co-authored-by: Ben Ye <[email protected]>

* Cut 0.23.0 release. (#4697)

* Endpointset: Do not use info client to obtain metadata (for now) (#4714)

* Do not use info client to obtain metadata

Signed-off-by: Matej Gera <[email protected]>

* Update CHANGELOG.

Signed-off-by: Matej Gera <[email protected]>

* Comment out client.info usage

Signed-off-by: Matej Gera <[email protected]>

* Fix lint error

Signed-off-by: Matej Gera <[email protected]>

* Cutting 0.23.1 (#4718)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Moved tutorials Thanos versions to 0.23.1

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Added volounteer for shepharding, fixed VERSION.

Signed-off-by: Bartlomiej Plotka <[email protected]>

Co-authored-by: Jimmiehan <[email protected]>
Co-authored-by: Ben Ye <[email protected]>
Co-authored-by: Matej Gera <[email protected]>
someshkoli pushed a commit to someshkoli/thanos that referenced this issue Nov 7, 2021
…tes config type (thanos-io#4657)

* Sidecar: Fix process external label when promethues v2.28+ use units.Bytes config type (thanos-io#4656)

Signed-off-by: hanjm <[email protected]>

* E2E: Upgrade prometheus image version

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