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

[patch] ✨ Add Stackdriver Monitoring, Tracing and Profiler support #479

Merged
merged 12 commits into from
Jun 18, 2020

Conversation

rinx
Copy link
Contributor

@rinx rinx commented Jun 16, 2020

Signed-off-by: Rintaro Okamura rintaro.okamura@gmail.com

Description:

  • Add Stackdriver Monitoring, Tracing and Profiler support to internal/observability package.
  • Updated detecting internal changes CI.
  • Add a filter for app_version_info metric labels, because Stackdriver Monitoring accepts only less than 10 labels.

Related Issue:

nothing.

How Has This Been Tested?:

nothing.

Environment:

  • Go Version: 1.14.3
  • Docker Version: 19.03.8
  • Kubernetes Version: 1.18.2
  • NGT Version: 1.11.5

Types of changes:

  • Bug fix [type/bug]
  • New feature [type/feature]
  • Add tests [type/test]
  • Security related changes [type/security]
  • Add documents [type/documentation]
  • Refactoring [type/refactoring]
  • Update dependencies [type/dependency]
  • Update benchmarks and performances [type/bench]
  • Update CI [type/ci]

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Checklist:

  • I have read the CONTRIBUTING document.
  • I have checked open Pull Requests for the similar feature or fixes?
  • I have added tests and benchmarks to cover my changes.
  • I have ensured all new and existing tests passed.
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly.

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jun 16, 2020

[WARNING:INTCFG] Changes in interal/config may require you to change Helm charts. Please check.

@codecov
Copy link

codecov bot commented Jun 16, 2020

Codecov Report

Merging #479 into master will decrease coverage by 0.03%.
The diff coverage is 2.18%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #479      +/-   ##
=========================================
- Coverage    8.07%   8.03%   -0.04%     
=========================================
  Files         396     394       -2     
  Lines       20320   20419      +99     
=========================================
  Hits         1640    1640              
- Misses      18449   18548      +99     
  Partials      231     231              
Impacted Files Coverage Δ
internal/config/observability.go 0.00% <0.00%> (ø)
...ternal/observability/collector/collector_option.go 4.34% <0.00%> (ø)
internal/observability/metrics/version/version.go 0.00% <0.00%> (ø)
internal/observability/observability.go 0.00% <0.00%> (ø)
internal/observability/observability_option.go 3.44% <0.00%> (-0.56%) ⬇️
.../observability/profiler/stackdriver/stackdriver.go 0.00% <0.00%> (ø)
...ability/profiler/stackdriver/stackdriver_option.go 7.24% <7.24%> (ø)
...rvability/exporter/prometheus/prometheus_option.go
...nal/observability/exporter/jaeger/jaeger_option.go
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1038c46...9699985. Read the comment docs.

@rinx rinx force-pushed the feature/observability/add-stackdriver-metrics-traces branch from 3eb16eb to 0f811d7 Compare June 16, 2020 03:28
@rinx rinx changed the title ✨ Add Stackdriver Monitoring and Tracing support ✨ Add Stackdriver Monitoring, Tracing and Profiler support Jun 16, 2020
@rinx rinx force-pushed the feature/observability/add-stackdriver-metrics-traces branch 2 times, most recently from 6adb44b to 0c3f7ff Compare June 16, 2020 08:12
internal/observability/profiler/stackdriver/stackdriver.go Outdated Show resolved Hide resolved
NumberOfWorkers int `json:"number_of_workers" yaml:"number_of_workers"`
}

type StackdriverProfiler struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
struct of size 96 bytes could be of size 88 bytes (maligned)

"go.opencensus.io/trace"
)

type Stackdriver interface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported type Stackdriver should have comment or be unexported (golint)

Profiler *StackdriverProfiler `json:"profiler" yaml:"profiler"`
}

type StackdriverExporter struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
struct of size 120 bytes could be of size 112 bytes (maligned)

"google.golang.org/api/option"
)

type Stackdriver interface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported type Stackdriver should have comment or be unexported (golint)

}
)

func WithProjectID(pid string) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithProjectID should have comment or be unexported (golint)

}
}

func WithService(name string) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithService should have comment or be unexported (golint)

}
}

func WithServiceVersion(version string) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithServiceVersion should have comment or be unexported (golint)

}
}

func WithDebugLogging(enabled bool) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithDebugLogging should have comment or be unexported (golint)

}
}

func WithMutexProfiling(enabled bool) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithMutexProfiling should have comment or be unexported (golint)

}
}

func WithCPUProfiling(enabled bool) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithCPUProfiling should have comment or be unexported (golint)

}
}

func WithAllocProfiling(enabled bool) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithAllocProfiling should have comment or be unexported (golint)

}
}

func WithHeapProfiling(enabled bool) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithHeapProfiling should have comment or be unexported (golint)

}
}

func WithGoroutineProfiling(enabled bool) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithGoroutineProfiling should have comment or be unexported (golint)

type Option func(e *exporter) error

var (
defaultOpts = []Option{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
defaultOpts is a global variable (gochecknoglobals)

@rinx rinx force-pushed the feature/observability/add-stackdriver-metrics-traces branch from bb6a05c to 0944eb5 Compare June 17, 2020 08:25
internal/observability/client/google/option.go Outdated Show resolved Hide resolved
internal/observability/client/google/option.go Outdated Show resolved Hide resolved
internal/observability/client/google/option.go Outdated Show resolved Hide resolved
internal/observability/client/google/option.go Outdated Show resolved Hide resolved
internal/observability/client/google/option.go Outdated Show resolved Hide resolved
internal/observability/client/google/option.go Outdated Show resolved Hide resolved
internal/observability/client/google/option.go Outdated Show resolved Hide resolved
@rinx rinx force-pushed the feature/observability/add-stackdriver-metrics-traces branch from 6db3513 to d43117b Compare June 17, 2020 08:53
@vdaas-ci
Copy link
Collaborator

[REBASE] Rebase triggered by rinx for branch: feature/observability/add-stackdriver-metrics-traces

@vdaas-ci vdaas-ci force-pushed the feature/observability/add-stackdriver-metrics-traces branch from a22ba7e to e525a26 Compare June 18, 2020 01:46
@vdaas-ci
Copy link
Collaborator

[FORMAT] Updating license headers and formatting go codes triggered by rinx.

@rinx rinx marked this pull request as ready for review June 18, 2020 01:52
"google.golang.org/api/option"
)

type Option = option.ClientOption
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported type Option should have comment or be unexported (golint)

// WithHTTPClient(client *http.Client) ClientOption
// WithTokenSource(s oauth2.TokenSource) ClientOption

func WithAPIKey(apiKey string) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithAPIKey should have comment or be unexported (golint)

return option.WithAPIKey(apiKey)
}

func WithAudiences(audiences ...string) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithAudiences should have comment or be unexported (golint)

return option.WithAudiences(audiences...)
}

func WithCredentialsFile(path string) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithCredentialsFile should have comment or be unexported (golint)

return option.WithCredentialsFile(path)
}

func WithEndpoint(endpoint string) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithEndpoint should have comment or be unexported (golint)

return option.WithRequestReason(rr)
}

func WithScopes(scopes ...string) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithScopes should have comment or be unexported (golint)

return option.WithScopes(scopes...)
}

func WithUserAgent(ua string) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithUserAgent should have comment or be unexported (golint)

return option.WithUserAgent(ua)
}

func WithCredentialsJSON(json string) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithCredentialsJSON should have comment or be unexported (golint)

return nil
}

func WithTelemetry(enabled bool) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithTelemetry should have comment or be unexported (golint)

return nil
}

func WithAuthentication(enabled bool) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
exported function WithAuthentication should have comment or be unexported (golint)

@rinx rinx requested a review from kpango June 18, 2020 06:36
Copy link
Collaborator

@kpango kpango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

others, LGTM

internal/observability/exporter/jaeger/jaeger.go Outdated Show resolved Hide resolved
internal/observability/exporter/prometheus/prometheus.go Outdated Show resolved Hide resolved
internal/observability/exporter/stackdriver/stackdriver.go Outdated Show resolved Hide resolved
internal/observability/observability.go Show resolved Hide resolved
internal/observability/observability.go Show resolved Hide resolved
@rinx
Copy link
Contributor Author

rinx commented Jun 18, 2020

@kpango Fixed. Please check them.

/rebase

@vdaas-ci
Copy link
Collaborator

[REBASE] Rebase triggered by rinx for branch: feature/observability/add-stackdriver-metrics-traces

rinx added 12 commits June 18, 2020 07:39
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>

:green_heart: Fix detecting internal changes

Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@vdaas-ci vdaas-ci force-pushed the feature/observability/add-stackdriver-metrics-traces branch from b7fe0ed to 9699985 Compare June 18, 2020 07:39
@kpango kpango changed the title ✨ Add Stackdriver Monitoring, Tracing and Profiler support [patch] ✨ Add Stackdriver Monitoring, Tracing and Profiler support Jun 18, 2020
@kpango
Copy link
Collaborator

kpango commented Jun 18, 2020

/format
/changelog
/approve

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jun 18, 2020

[CHANGELOG] Please edit the following lines.

  • ✨ Add Stackdriver Monitoring, Tracing and Profiler support (#479)
  • Add internal/params pacakge test (#474)
  • ✨ aws region can be specified with empty string (#477)
  • Fix failed test case of internal/safety package (#464)
  • internal/db/storage/blob/s3: remove ctx from struct (#473)

@kpango kpango merged commit 42577f9 into master Jun 18, 2020
@kpango kpango deleted the feature/observability/add-stackdriver-metrics-traces branch June 18, 2020 07:49
@vdaas-ci
Copy link
Collaborator

[FORMAT] Updating license headers and formatting go codes triggered by kpango.

@vdaas-ci
Copy link
Collaborator

[FORMAT] Failed to format.

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

Successfully merging this pull request may close these issues.

3 participants