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

Remove Prometheus dependency for push pkg #9937

Merged
merged 3 commits into from
Jul 17, 2023

Conversation

salvacorts
Copy link
Contributor

@salvacorts salvacorts commented Jul 14, 2023

What this PR does / why we need it:

In #9694, we modified the push pkg to import Prometheus as a dependency to use the labels.Labels type for the entries' non-indexed labels.

Having Prometheus as a dependency is problematic since any project importing the push pkg will need to import Prometheus as a result. In fact, this is one of the reasons why the push pkg was extracted from Loki in the first place (#8259).

This PR removes the dependency of Prometheus from the push pkg by copying some bits of the implementation for Prometheus' labels.Labels. We copy:

  • The Labels struct definition
  • The JSON Marshaling and Unmarshaling methods for the labels. We need this so labels are encoded as maps instead of an array of objects.

Notes for reviewers:

  • To implement the JSON Marshaling and Unmarshaling methods the push pkg now depends on golang.org/x/exp. I think it should be fine for projects importing the push pkg to also depend on golang.org/x/exp, right?

@pull-request-size pull-request-size bot added size/M and removed size/L labels Jul 14, 2023
@salvacorts salvacorts marked this pull request as ready for review July 14, 2023 08:27
@salvacorts salvacorts requested a review from a team as a code owner July 14, 2023 08:27
Copy link
Contributor

@vlad-diachenko vlad-diachenko left a comment

Choose a reason for hiding this comment

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

lgtm

@salvacorts salvacorts merged commit 15af77b into main Jul 17, 2023
5 checks passed
@salvacorts salvacorts deleted the salvacorts/remove-prometheus-dep-push-pkg branch July 17, 2023 06:43
@grafanabot
Copy link
Collaborator

Hello @vlad-diachenko!
Backport pull requests need to be either:

  • Pull requests which address bugs,
  • Urgent fixes which need product approval, in order to get merged,
  • Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If the pull request modifies CI behaviour, please add the type/ci label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

@vlad-diachenko vlad-diachenko added type/bug Somehing is not working as expected backport k159 and removed backport k159 labels Jul 17, 2023
grafanabot pushed a commit that referenced this pull request Jul 17, 2023
**What this PR does / why we need it**:

In #9694, we modified the `push` pkg
to import Prometheus as a dependency to use the `labels.Labels` type for
the entries' non-indexed labels.

Having Prometheus as a dependency is problematic since any project
importing the `push` pkg will need to import Prometheus as a result. In
fact, this is one of the reasons why the `push` pkg was extracted from
Loki in the first place (#8259).

This PR removes the dependency of Prometheus from the `push` pkg by
copying some bits of the implementation for Prometheus' `labels.Labels`.
We copy:
- The Labels struct definition
- The JSON Marshaling and Unmarshaling methods for the labels. We need
this so labels are encoded as maps instead of an array of objects.

---

**Notes for reviewers:**

- To implement the JSON Marshaling and Unmarshaling methods the `push`
pkg now depends on `golang.org/x/exp`. I think it should be fine for
projects importing the `push` pkg to also depend on `golang.org/x/exp`,
right?

(cherry picked from commit 15af77b)
vlad-diachenko pushed a commit that referenced this pull request Jul 17, 2023
Backport 15af77b from #9937

---

**What this PR does / why we need it**:

In #9694, we modified the `push` pkg
to import Prometheus as a dependency to use the `labels.Labels` type for
the entries' non-indexed labels.

Having Prometheus as a dependency is problematic since any project
importing the `push` pkg will need to import Prometheus as a result. In
fact, this is one of the reasons why the `push` pkg was extracted from
Loki in the first place (#8259).

This PR removes the dependency of Prometheus from the `push` pkg by
copying some bits of the implementation for Prometheus' `labels.Labels`.
We copy:
- The Labels struct definition
- The JSON Marshaling and Unmarshaling methods for the labels. We need
this so labels are encoded as maps instead of an array of objects.

---

**Notes for reviewers:**

- To implement the JSON Marshaling and Unmarshaling methods the `push`
pkg now depends on `golang.org/x/exp`. I think it should be fine for
projects importing the `push` pkg to also depend on `golang.org/x/exp`,
right?

Co-authored-by: Salva Corts <salva.corts@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport k159 size/M type/bug Somehing is not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants