yabeda icon indicating copy to clipboard operation
yabeda copied to clipboard

Current handling of groups leaves room for user error/confusion

Open Envek opened this issue 3 years ago • 2 comments

Given the current implementation of group method for DSL: https://github.com/yabeda-rb/yabeda/blob/bd3b49fe0fd293fe6ac2d29f9e04fa5bdde79b0c/lib/yabeda/dsl/class_methods.rb#L31-L37

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 https://github.com/yabeda-rb/yabeda/issues/19#issuecomment-821183885

Envek avatar Apr 16 '21 14:04 Envek

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 avatar Apr 16 '21 15:04 Envek

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)

Envek avatar Apr 16 '21 15:04 Envek