-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[cmd/mdatagen] Mdatagen support histogram metrics #26558
[cmd/mdatagen] Mdatagen support histogram metrics #26558
Conversation
Occasional e2e-tests(kubernetes-test) failures, hello @atoulme seems to PR restart job on failure try to solve that. |
func (m *metric{{ $name.Render }}) recordDataPoint(start pcommon.Timestamp, ts pcommon.Timestamp, val {{ $metric.Data.MetricValueType.BasicType }} | ||
{{- range $metric.Attributes -}}, {{ .RenderUnexported }}AttributeValue {{ (attributeInfo .).Type.Primitive }}{{ end }}) { | ||
func (m *metric{{ $name.Render }}) recordDataPoint(start pcommon.Timestamp, ts pcommon.Timestamp | ||
{{- if eq $metric.Data.Type "Histogram" }}, count uint64, sum float64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about bucket values? Histograms without buckets are invalid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Thanks for reviewing my code. I realize that I overlooked the details in docs data-model_Histogram, spec_Histogram and proto_Histogram
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @dmitryax , would you be available to help review the code again if possible, i would like to assist in resolving this issue, thanks
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Could this PR hold on? |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
28459d8
to
69ed578
Compare
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me, @dmitryax should review as codeowner.
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
69ed578
to
7eaa323
Compare
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Description:
Metadata generator support histogram metrics
Link to tracking Issue:
#26118
Testing:
make generate
make mdatagen-test
make chlog-validate
Documentation:
cmd/mdatagen/documentation.md