Skip to content

Optionally print OM created lines#1408

Merged
bwplotka merged 1 commit intomainfrom
om-created-lines
Oct 14, 2024
Merged

Optionally print OM created lines#1408
bwplotka merged 1 commit intomainfrom
om-created-lines

Conversation

@ArthurSens
Copy link
Member

@ArthurSens ArthurSens commented Dec 12, 2023 •
edited
Loading

This PR adds an extra option to print _created lines when OpenMetrics is the chosen exposition format. It's implemented following the conditional option added in prometheus/common#504.

@ArthurSens ArthurSens changed the title Optionally print OM created lines [WIP] Optionally print OM created lines Dec 12, 2023
bwplotka
bwplotka reviewed Dec 22, 2023
View reviewed changes
@ArthurSens ArthurSens mentioned this pull request Feb 2, 2024
Merged
@ArthurSens ArthurSens changed the title [WIP] Optionally print OM created lines Optionally print OM created lines Feb 2, 2024
@ArthurSens ArthurSens force-pushed the om-created-lines branch 3 times, most recently from e930252 to 240345e Compare February 2, 2024 18:35
bwplotka
bwplotka approved these changes Feb 5, 2024
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Thanks! One minor nit, otherwise well done!

@ArthurSens ArthurSens force-pushed the om-created-lines branch 2 times, most recently from 33ed3e6 to e9c6f77 Compare February 18, 2024 18:33
bwplotka
bwplotka approved these changes Mar 25, 2024
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Epic, proposed some improvements, thanks!

@ArthurSens
Copy link
Member Author

ArthurSens commented Mar 25, 2024

We need prometheus/common v0.49.0 for this feature, but this version of prometheus/common requires go 1.21.

Therefore, the PR is blocked until we drop support for go 1.20

@ArthurSens ArthurSens added the OpenMetrics OpenMetrics v1 label Mar 25, 2024
@ArthurSens ArthurSens mentioned this pull request Mar 28, 2024
Open
kakkoyun
kakkoyun approved these changes Apr 8, 2024
View reviewed changes
Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

LGTM

@ArthurSens
Copy link
Member Author

ArthurSens commented Apr 11, 2024

After quite some work to unblock this, this is finally ready :)

Do you want to review again or should I just merge?

@ArthurSens ArthurSens mentioned this pull request Apr 11, 2024
Open
@ArthurSens
Copy link
Member Author

ArthurSens commented Aug 12, 2024

Now that prometheus/prometheus#14356 was merged, should we revive this? :)

@ArthurSens ArthurSens mentioned this pull request Aug 19, 2024
Merged
@ArthurSens ArthurSens force-pushed the om-created-lines branch 2 times, most recently from eb0f4ef to b51b530 Compare September 2, 2024 17:01
@ArthurSens
Copy link
Member Author

ArthurSens commented Sep 2, 2024

PR rebased on main. Ready for another review just in case @kakkoyun @bwplotka @vesari

@ArthurSens
Copy link
Member Author

ArthurSens commented Sep 5, 2024

Since we have approvals from the past, I'll take the liberty to merge this PR next week :)

bwplotka
bwplotka requested changes Sep 18, 2024
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Let's timebox this, but wanted to try last more time to consider an improvement to the config structure, otherwise LGTM!

Maniktherana
Maniktherana reviewed Sep 20, 2024
View reviewed changes
Copy link

@Maniktherana Maniktherana left a comment •
edited
Loading

Choose a reason for hiding this comment

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

should we add summaries and counters in the example too?

This was referenced Feb 18, 2026
Closed
Closed
Merged
Closed
@renovate renovate bot mentioned this pull request Feb 19, 2026
Open
1 task
@konflux-internal-p02 konflux-internal-p02 bot mentioned this pull request Feb 20, 2026
Open
1 task
@renovate renovate bot mentioned this pull request Feb 21, 2026
Open
1 task
@soerenschneider soerenschneider mentioned this pull request Feb 24, 2026
Merged
1 task
This was referenced Feb 24, 2026
Merged
Closed
Open
This was referenced Mar 3, 2026
Closed
Closed
Closed
Closed
Closed
@renovate renovate bot mentioned this pull request Mar 4, 2026
Open
1 task
@renovate-sh-app renovate-sh-app bot mentioned this pull request Mar 5, 2026
Open
1 task
This was referenced Mar 5, 2026
Open
Merged
Merged
@renovate-bot renovate-bot mentioned this pull request Mar 12, 2026
Open
1 task
This was referenced Mar 13, 2026
Merged
Open
Open
Open
Closed
Open
This was referenced Mar 23, 2026
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vesari vesari vesari left review comments

@kakkoyun kakkoyun kakkoyun approved these changes

@bwplotka bwplotka bwplotka approved these changes

@krajorama krajorama krajorama approved these changes

+1 more reviewer

@Maniktherana Maniktherana Maniktherana left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

OpenMetrics OpenMetrics v1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ArthurSens @kakkoyun @bwplotka @krajorama @Maniktherana @vesari