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

Unexpected TimeSeriesLimitReached after HistogramLimitReached #60752

Closed
itn3000 opened this issue Oct 22, 2021 · 5 comments
Closed

Unexpected TimeSeriesLimitReached after HistogramLimitReached #60752

itn3000 opened this issue Oct 22, 2021 · 5 comments

Comments

@itn3000
Copy link
Contributor

itn3000 commented Oct 22, 2021

Description

Unexpected TimeSeriesLimitReached after HistogramLimitReached and trying to add more histogram instrument,
and no more Instrument can be added in session.

Reproduction Steps

  1. begin to listen EventSource named "System.Diagnostics.Metrics" with "MaxTimeSeries" and "MaxHistograms" options
    • must be MaxTimeSeries > MaxHistograms(temporarily set MaxTimeSeries=3, MaxHistograms=2)
  2. create System.Diagnostics.Metrics.Meter
  3. create Histogram via it
  4. do Histogram.Record 3 times with different tag values(temporarily, "t1=1", "t1=2", "t1=3")
  5. do Histogram.Record with tag value which is same as last record(temporarily, "t1=3")

sample program is following
https://gist.github.com/itn3000/9001546f1a0a9bcd53b7bc0ae4184bbd

Expected behavior

raise "HistogramLimitReached" only, no "TimeSeriesLimitReached".
and another Instrument like Counter<T> can be started to listen after "HistogramLimitReached".

Actual behavior

raise "HistogramLimitReached" and "TimeSeriesLimitReached".
and no more Instrument can be started to listen after "TimeSeriesLimitReached"

Regression?

No response

Known Workarounds

  • make "MaxHistograms" large to exceed possible histogram number
  • avoiding do Histogram.Record which not listening(if it can)

Configuration

  • dotnet-sdk: 6.0.0-rc2
  • OS: win10-x64

Other information

I suspect around the "CheckTimeSeriesAllowed" and "CheckHistramAllowed"(currentTimeSeries is increased even if after reaching MaxHistograms?)

return (!CheckTimeSeriesAllowed() || !CheckHistogramAllowed()) ?
null :
new ExponentialHistogramAggregator(s_defaultHistogramConfig);

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Oct 22, 2021
@itn3000
Copy link
Contributor Author

itn3000 commented Oct 25, 2021

I think CheckTimeSeriesAllowed and CheckHistogramAllowed order should be swapped.

return (!CheckTimeSeriesAllowed() || !CheckHistogramAllowed()) ?

@itn3000
Copy link
Contributor Author

itn3000 commented Nov 4, 2021

I make PR for this issue(#61199)

@tarekgh tarekgh removed the untriaged New issue has not been triaged by the area owner label Nov 4, 2021
@tarekgh tarekgh added this to the 7.0.0 milestone Nov 4, 2021
@tarekgh
Copy link
Member

tarekgh commented Nov 4, 2021

CC @noahfalk

itn3000 added a commit to itn3000/runtime that referenced this issue Nov 5, 2021
@tarekgh
Copy link
Member

tarekgh commented Nov 5, 2021

This is already fixed by the linked PR. Thanks to @itn3000 for helping with this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants