[ 
https://issues.apache.org/jira/browse/BEAM-11217?focusedWorklogId=675904&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-675904
 ]

ASF GitHub Bot logged work on BEAM-11217:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 04/Nov/21 01:39
            Start Date: 04/Nov/21 01:39
    Worklog Time Spent: 10m 
      Work Description: lostluck commented on a change in pull request #15887:
URL: https://github.com/apache/beam/pull/15887#discussion_r742418826



##########
File path: sdks/go/pkg/beam/core/metrics/metrics.go
##########
@@ -843,13 +872,16 @@ func ResultsExtractor(ctx context.Context) Results {
                        committed[key] = GaugeValue{opt.(*gauge).v, 
opt.(*gauge).t}
                        r.gauges = append(r.gauges, MergeGauges(attempted, 
committed)...)
                case *executionState:
-                       for _, v := range opt.(*executionState).state {
-                               attempted := make(map[StepKey]MsecValue)
-                               committed := make(map[StepKey]MsecValue)
-                               attempted[key] = MsecValue{}
-                               committed[key] = MsecValue{int64(v.TotalTime)}
-                               r.msecs = append(r.msecs, MergeMsecs(attempted, 
committed)...)
+                       attempted := make(map[StepKey]MsecValue)
+                       committed := make(map[StepKey]MsecValue)
+                       attempted[key] = MsecValue{}
+                       committed[key] = MsecValue{[4]time.Duration{0, 0, 0, 0}}

Review comment:
       Nit: New allocations for Go values are always their default 0 value. So 
`MsecValue{}` and  `MsecValue{[4]time.Duration{0, 0, 0, 0}}` are identical.

##########
File path: sdks/go/pkg/beam/core/metrics/metrics.go
##########
@@ -481,7 +481,7 @@ func (m *executionState) kind() kind {
 
 // MsecValue is the value of a single msec metric
 type MsecValue struct {
-       Time int64
+       Time [4]time.Duration

Review comment:
       Consider the following:
   We change the type name to DoFnTimes.
   
   Then instead of an array that doesn't indicate any semantics to a user 
(which has the total? 0? 3?) we expand those to fields.
   
   ```
   type DoFnTimes struct{
     Start, Process, Finish, Total time.Duration
   }
   ```
   
   This makes it easier for users to understand what they're getting.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 675904)
    Time Spent: 9.5h  (was: 9h 20m)

> Implement metrics filtering
> ---------------------------
>
>                 Key: BEAM-11217
>                 URL: https://issues.apache.org/jira/browse/BEAM-11217
>             Project: Beam
>          Issue Type: Sub-task
>          Components: sdk-go
>            Reporter: Kamil Wasilewski
>            Assignee: Ritesh Ghorse
>            Priority: P3
>              Labels: stale-assigned
>          Time Spent: 9.5h
>  Remaining Estimate: 0h
>
> `metrics.Results` misses a method for querying metrics using a provided 
> filter. The method should take a filter object as an argument and return 
> `metrics.QueryResults` object containing metrics that matched the filter.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to