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

Current handling of groups leaves room for user error/confusion #20

Open
Envek opened this issue Apr 16, 2021 · 2 comments
Open

Current handling of groups leaves room for user error/confusion #20

Envek opened this issue Apr 16, 2021 · 2 comments

Comments

@Envek
Copy link
Member

Envek commented Apr 16, 2021

Given the current implementation of group method for DSL:

def group(group_name)
@group = group_name
return unless block_given?
yield
@group = nil
end

We can configure sibling groups

Yabeda.configure do
  group :g1
  # config
  group :g2
  # config
  end

or attempt to have nested groups

Yabeda.configure do
  group :g1 do
    # config
    group :g2 do
      # config
    end
    # more g1 config 
  end 

The nested groups are really the same as the first situation where we create two siblings. Both default_tags and looking up metrics would be handled the same in both cases with an additional surprise that the 'more g1 config'j would apply to the global scope.

I think that it would be good to raise an exception or at least warn when someone attempts to nest two groups to prevent the issue until there is a need for deeper nesting?

Originally posted by @liaden in #19 (comment)

@Envek
Copy link
Member Author

Envek commented Apr 16, 2021

We can configure sibling groups:

Yabeda.configure do
  group :g1
  # metrics, default tags, etc
  group :g2
  # metrics, default tags, etc
end

I think that this is absolutely fine. Hard to maintain in gem code, but while group contents are reasonably small (fits into single screen) it is both understandable and clean.

@Envek
Copy link
Member Author

Envek commented Apr 16, 2021

Yabeda.configure do
  group :g1 do
    # config
    group :g2 do
      # config
    end
    # more g1 config 
  end

That one implies that nesting groups are supported (which is not true). Here, IMO, we should raise an exception. Or add support for nested groups (however no one has ever asked for this AFAIR)

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

No branches or pull requests

1 participant