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

core: Add option to merge families #286

Merged
merged 1 commit into from
Oct 25, 2019
Merged

core: Add option to merge families #286

merged 1 commit into from
Oct 25, 2019

Conversation

gjasny
Copy link
Collaborator

@gjasny gjasny commented Jul 6, 2019

No description provided.

@gjasny gjasny requested a review from jupp0r July 6, 2019 20:03
core/include/prometheus/registry.h Outdated Show resolved Hide resolved
core/include/prometheus/registry.h Show resolved Hide resolved
core/include/prometheus/family.h Outdated Show resolved Hide resolved
@gjasny gjasny requested a review from jupp0r July 13, 2019 17:40
@gjasny gjasny changed the title [WIP] core: Add option to merge families core: Add option to merge families Jul 13, 2019
@gjasny gjasny force-pushed the merge-same-families branch 2 times, most recently from 7d9b25d to 42b93d1 Compare July 13, 2019 20:51
std::lock_guard<std::mutex> lock{mutex_};

if (insert_behavior_ == InsertBehavior::Merge) {
for (auto &family : families) {
Copy link
Owner

Choose a reason for hiding this comment

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

use std::find_if

core/include/prometheus/registry.h Outdated Show resolved Hide resolved
@@ -36,6 +36,20 @@ namespace prometheus {
/// a data race.
class Registry : public Collectable {
public:
/// \brief How to deal with repeatedly added family names for a type
enum class InsertBehavior {
/// \brief Create new family object and append
Copy link
Owner

Choose a reason for hiding this comment

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

Can you describe more verbosely how the algorithm works? What happens when there is no existing family with that name, what happens when there is an existing family with that name, but a different type, etc?

Copy link
Owner

Choose a reason for hiding this comment

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

ie in the doc comment for each behavior

@gjasny gjasny force-pushed the merge-same-families branch 2 times, most recently from f4dddcd to 1d91f0c Compare October 18, 2019 15:51
@jupp0r
Copy link
Owner

jupp0r commented Oct 22, 2019

Just realized that the commit message needs to be updated.

Add three different insert strategies for the Registry.
The `NonStandardAppend` is only there and selected for
backward compatibility and most likely will go away
in the future.
@gjasny gjasny merged commit 8b5b03c into master Oct 25, 2019
@gjasny gjasny deleted the merge-same-families branch February 29, 2020 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants