Add tracing to fs ops #2371
Add tracing to fs ops #2371
Conversation
kislaykishore
added
execute-perf-test
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2371 +/- ##
==========================================
+ Coverage 79.03% 79.07% +0.04%
==========================================
Files 105 105
Lines 11654 11577 -77
==========================================
- Hits 9211 9155 -56
+ Misses 1978 1958 -20
+ Partials 465 464 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
kislaykishore
requested review from
a team and
gargnitingoogle
and removed request for
a team
| } | ||
|
|
||
| err := m.StatFS(context.Background(), nil) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
how is this different from assert.NoError ? If no difference, let's use assert.NoError for consistency.
|
|
||
| func (d dummyFS) Destroy() {} | ||
|
|
||
| func TestSpan(t *testing.T) { |
There was a problem hiding this comment.
The name is a bit unclear to me. This sounds like it's testing the span functionality, which is not a gcsfuse feature.. How about TestInvocationSpan ?
| return nil | ||
| } | ||
|
|
||
| func (d dummyFS) Destroy() {} |
There was a problem hiding this comment.
Given that Destroy is the only function of FS which is different from the rest in signature, and destroy is also interesting from deinitialization point of view, I suggest we add a test with it, or update the existing test with a call to m.Destroy() to see that it gets recorded by monitoring.
|
My bad. I guess this PR was open in a tab in my browser for a few hours before I got to it, and didn't realize that it had been approved and merged already. |
Description
Link to the issue in case of a bug fix.
NA
Testing details