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

Minimal working version #3

Merged
merged 1 commit into from
May 16, 2024
Merged

Conversation

olegbespalov
Copy link
Collaborator

What?

It's the first minimum working version that actually sends metrics to an OTEl receiver.

It has limited support of metric types (count, gauge, trend(?)). Only gRPC receiver. Limited configuration.

Why?

An iterative approach to reduce the scope of the PRs.

Dependent on #2

Part of #1

@olegbespalov olegbespalov self-assigned this May 15, 2024
@olegbespalov olegbespalov requested a review from a team as a code owner May 15, 2024 13:55
@olegbespalov olegbespalov requested review from oleiade and codebien and removed request for a team May 15, 2024 13:55
@olegbespalov olegbespalov force-pushed the feat/minimal-working-version branch from 0c70256 to c40a57c Compare May 15, 2024 13:57
Copy link

@codebien codebien left a comment

Choose a reason for hiding this comment

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

General question, do we expect to start delta or cumulative mode? Or both?

@@ -36,47 +52,198 @@ func New(p output.Params) (*Output, error) {

// Description returns a human-readable description of the output that will be shown in `k6 run`
func (o *Output) Description() string {
return "OpenTelemetry: " + o.config.Address
return "opentelemetry: " + o.config.String()
}

// Stop flushes all remaining metrics and finalizes the test run
func (o *Output) Stop() error {

Choose a reason for hiding this comment

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

Suggested change
func (o *Output) Stop() error {
func (o *Output) StopWithTestError() error {

We should implement StopWithTestError instead of Stop(). Stop should just proxy the request.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure if I'm getting the idea or maybe missing the context; Stop is part of the interface. StopWithTestError is planned to replace Stop ?

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch then, I'll adjust accordingly 👍

}

// Stop flushes all remaining metrics and finalizes the test run
func (o *Output) Stop() error {
o.logger.Debug("Stopping...")
defer o.logger.Debug("Stopped!")

if err := o.meterProvider.Shutdown(context.Background()); err != nil {

Choose a reason for hiding this comment

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

Should we consider forcing a timeout here? Or do we rely on the fact that the client will have one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we could introduce a configuration in a later PRs 👍

pkg/opentelemetry/output.go Show resolved Hide resolved
pkg/opentelemetry/output.go Outdated Show resolved Hide resolved
pkg/opentelemetry/output.go Outdated Show resolved Hide resolved
Base automatically changed from chore/otel-docker-compose to main May 15, 2024 16:01
@olegbespalov olegbespalov force-pushed the feat/minimal-working-version branch from c40a57c to a477b15 Compare May 15, 2024 16:09
@olegbespalov
Copy link
Collaborator Author

General question, do we expect to start delta or cumulative mode? Or both?

@codebien If possible, we could make this configurable, so probably both. However, the default behavior should be close to most of the other k6 outputs.

@olegbespalov olegbespalov requested a review from codebien May 15, 2024 16:12
@olegbespalov olegbespalov force-pushed the feat/minimal-working-version branch from a477b15 to 2bc64c6 Compare May 16, 2024 06:28
@olegbespalov olegbespalov requested a review from codebien May 16, 2024 06:32
It has limited support of metric types (count, gauge, trend(?)). Only
gRPC receiver. Limited configuration.
@olegbespalov olegbespalov force-pushed the feat/minimal-working-version branch from 2bc64c6 to 39d991c Compare May 16, 2024 08:29
@olegbespalov olegbespalov requested a review from codebien May 16, 2024 08:43
@olegbespalov olegbespalov merged commit 8ddf863 into main May 16, 2024
10 checks passed
@olegbespalov olegbespalov deleted the feat/minimal-working-version branch May 16, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants