Tags set with Peastash.with_instance.tags are not scoped to the instance

[alx75]

Context

I don't know if it's by design but it feels wrong having tags scoped on store_name [1]. This means tags are shared across instances when using the same store name which is peastash by default.

How to reproduce

[1] pry(main)> Peastash.with_instance.tags << 'global'
=> ["global"]
[2] pry(main)> Peastash.with_instance.tags
=> ["global"]
[3] pry(main)> Peastash.with_instance(:test).tags
=> ["global"]
[4] pry(main)> Peastash.with_instance(:test).tags << 'test'
=> ["global", "test"]
[5] pry(main)> Peastash.with_instance.tags
=> ["global", "test"]
[6] pry(main)> Peastash.with_instance.log {}
=> 136
[7] pry(main)> Peastash.with_instance(:test).tags 
=> []

At step 3 we can see that the test instance also have the tag set.
At stage 7 we can see that the tag test is removed from instance test.

It's even worst if tags are passed to log method.

[1] pry(main)> Peastash.with_instance(:first).log(['first-tag']) {}
=> 147
[2] pry(main)> Peastash.with_instance(:other).tags
=> ["first-tag"]

The tag passed to the log method in first instance leaks into the :other instance.

Expected output

[1] pry(main)> Peastash.with_instance.tags << 'global'
=> ["global"]
[2] pry(main)> Peastash.with_instance.tags
=> ["global"]
[3] pry(main)> Peastash.with_instance(:test).tags
=> []
[4] pry(main)> Peastash.with_instance(:test).tags << 'test'
=> ["test"]
[5] pry(main)> Peastash.with_instance.tags
=> ["global"]
[6] pry(main)> Peastash.with_instance.log {}
=> 136
[7] pry(main)> Peastash.with_instance(:test).tags 
=> ["test]

Workaround
Use different store name per instance.

[1]

  def tags
    Peastash.safely do
      configure! unless configured?
      Thread.current[@store_name + ":tags"] ||= []
    end
  end

[elhu]

Thanks for the report, I'll try to take a look over the weekend.

[alx75]

Thanks for taking the time to have a look. Let me know if I can help.

[elhu]

So, in practice it shouldn't be a big issue because the tags will get cleared the next time the #log method is called.

That said, the behaviour is confusing and I think I can make it better. I have a patch working, but I need to strengthen the tests before rollout.

As some existing integrations could be relying on this behaviour, this will be rolled out as a major version.

[elhu]

@alx75 could you give this branch a shot, see if it fixes your issue?

[alx75]

Thanks a lot for the PR. I had a look at it and dropped a #22 (comment) . I'll give it a try when times allow it.