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

feat: add support for getting DORA metrics from projects and groups #1582

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

2785
Copy link
Contributor

@2785 2785 commented Nov 26, 2022

resolves #1557

Copy link

@jimmy0012 jimmy0012 left a comment

Choose a reason for hiding this comment

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

Was testing this and ran into an issue. Sometimes Dora metrics are not integers so these functions will not be able to marshal the json:
error="error fetching DORA Metric leadTime for <repo>: json: cannot unmarshal number 607.5 into Go struct field DORAMetric.value of type int" project-name=<repo>

@2785
Copy link
Contributor Author

2785 commented Dec 1, 2022

😐 I so wish there's a day that gitlab docs actually tell you that

type DORAMetricInterval string

const (
// daily is the default
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// daily is the default

"net/http"
)

type DORAMetricType string
Copy link
Member

Choose a reason for hiding this comment

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

Please add comments on all exported types and methods such that my linters don't fall over this...

DORAMetricIntervalDaily DORAMetricInterval = "daily"
DORAMetricIntervalMonthly DORAMetricInterval = "monthly"
DORAMetricIntervalAll DORAMetricInterval = "all"
)
Copy link
Member

Choose a reason for hiding this comment

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

Can you please move these types to types.go and also add helper methods to create pointers to these types/consts? See types.go for examples of what I mean.

}

type GetDORAMetricsOptions struct {
Metric DORAMetricType `url:"metric" json:"metric"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Metric DORAMetricType `url:"metric" json:"metric"`
Metric *DORAMetricType `url:"metric,omitempty" json:"metric,omitempty"`

Metric DORAMetricType `url:"metric" json:"metric"`
EndDate *string `url:"end_date,omitempty" json:"end_date,omitempty"`
EnvironmentTiers *[]string `url:"environment_tiers,omitempty" del:"," json:"environment_tiers,omitempty"`
// Interval can be one of daily, monthly, all, default is daily
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Interval can be one of daily, monthly, all, default is daily

EnvironmentTier *string `url:"environment_tier,omitempty" json:"environment_tier,omitempty"`
}

// DORAMetricsService handles communication with the DORA metrics related methods of the GitLab API
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// DORAMetricsService handles communication with the DORA metrics related methods of the GitLab API
// DORAMetricsService handles communication with the DORA metrics related methods
// of the GitLab API


// GetProjectDORAMetrics gets the DORA metrics for a project.
//
// GitLab API Docs: https://docs.gitlab.com/ee/api/dora/metrics.html#get-project-level-dora-metrics
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// GitLab API Docs: https://docs.gitlab.com/ee/api/dora/metrics.html#get-project-level-dora-metrics
// GitLab API Docs:
// https://docs.gitlab.com/ee/api/dora/metrics.html#get-project-level-dora-metrics

Comment on lines +65 to +69

if opt.Metric == "" {
return nil, nil, fmt.Errorf("metric cannot be empty")
}

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if opt.Metric == "" {
return nil, nil, fmt.Errorf("metric cannot be empty")
}

While I understand this, I prefer to let the API return this info instead.


// GetGroupDORAMetrics gets the DORA metrics for a group.
//
// GitLab API Docs: https://docs.gitlab.com/ee/api/dora/metrics.html#get-group-level-dora-metrics
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// GitLab API Docs: https://docs.gitlab.com/ee/api/dora/metrics.html#get-group-level-dora-metrics
// GitLab API Docs:
// https://docs.gitlab.com/ee/api/dora/metrics.html#get-group-level-dora-metrics

Comment on lines +94 to +98

if opt.Metric == "" {
return nil, nil, fmt.Errorf("metric cannot be empty")
}

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if opt.Metric == "" {
return nil, nil, fmt.Errorf("metric cannot be empty")
}

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.

Feature Request API add support for DORA metrics
3 participants