-
Notifications
You must be signed in to change notification settings - Fork 66
Adding copilot metrics #487
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
base: main
Are you sure you want to change the base?
Conversation
|
Hey @austenstone 👋 Thanks for this PR. It's amazing work and I'm planning to review it throughly. But I noticed that you accidently pushed package-lock.json file. Could you please remove it? |
@itsmylife I honestly did all of this with Claude sonnet to get the ball rolling. I did review and test that it worked but I think the UI for data selection needs some TLC. Glad to hear you're opening to accepting the PR. I'll keep working on it. This functionality will help a lot of people! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't finished my review yet but I just want to share these. After testing it with proper access token and check backend code throughly I'll share another feedback.
Thank you very much for sparing time for this feature!
<Input | ||
aria-label={components.QueryEditor.Owner.input} | ||
width={RightColumnWidth} | ||
value={owner} | ||
onChange={(el) => setOwner(el.currentTarget.value)} | ||
onBlur={(el) => | ||
props.onChange({ | ||
...props, | ||
owner: el.currentTarget.value, | ||
}) | ||
} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to define this expilicitly. In QueryEditor QueryType.Copilot_Metrics
must be removed from nonRepoTypes
.
}; | ||
|
||
const nonRepoTypes = [QueryType.Projects, QueryType.ProjectItems]; | ||
const nonRepoTypes = [QueryType.Projects, QueryType.ProjectItems, QueryType.Copilot_Metrics]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const nonRepoTypes = [QueryType.Projects, QueryType.ProjectItems, QueryType.Copilot_Metrics]; | |
const nonRepoTypes = [QueryType.Projects, QueryType.ProjectItems]; |
|
||
const QueryEditorCopilotMetrics = (props: Props) => { | ||
const [team, setTeam] = useState(props.teamSlug || ''); | ||
const [owner, setOwner] = useState<string>(props.owner || ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need this one since we'll remove owner input component
organization?: string; | ||
teamSlug?: string; | ||
since?: string; | ||
until?: string; | ||
page?: number; | ||
perPage?: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't use this many fields. In the editor only teamSlug
is being used. The rest is useless for now.
Since: opt.Since, | ||
Until: opt.Until, | ||
Page: opt.Page, | ||
PerPage: opt.PerPage, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are not passed from frontend for now so we can remove them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't test it because I don't have an organization which has copilot metrics and has permission to see them. But I left a bunch of comments.
frameName := "copilot_metrics" | ||
if opts.TeamSlug != "" { | ||
frameName = "copilot_metrics_team" | ||
} | ||
|
||
return copilotMetricsToDataFrame(CopilotMetricsResponse(metrics), frameName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need these I believe. Just returning metrics
would be enough. Frames()
method will handle this part. So we don't need copilotMetricsToDataFrame
too
// ListCopilotMetricsOptions defines the options for listing Copilot metrics for an organization or team | ||
type ListCopilotMetricsOptions struct { | ||
Organization string `json:"organization"` | ||
TeamSlug string `json:"team_slug,omitempty"` | ||
Since *time.Time `json:"since,omitempty"` | ||
Until *time.Time `json:"until,omitempty"` | ||
Page int `json:"page,omitempty"` | ||
PerPage int `json:"per_page,omitempty"` | ||
} | ||
|
||
// CopilotMetrics represents a daily metrics record for Copilot usage | ||
type CopilotMetrics struct { | ||
Date string `json:"date"` | ||
TotalActiveUsers int `json:"total_active_users"` | ||
TotalEngagedUsers int `json:"total_engaged_users"` | ||
CopilotIDECodeCompletions CopilotIDECodeCompletions `json:"copilot_ide_code_completions"` | ||
CopilotIDEChat CopilotIDEChat `json:"copilot_ide_chat"` | ||
CopilotDotcomChat CopilotDotcomChat `json:"copilot_dotcom_chat"` | ||
CopilotDotcomPullRequests CopilotDotcomPullRequests `json:"copilot_dotcom_pull_requests"` | ||
} | ||
|
||
// CopilotIDECodeCompletions represents code completion metrics in IDEs | ||
type CopilotIDECodeCompletions struct { | ||
TotalEngagedUsers int `json:"total_engaged_users"` | ||
Languages []CopilotLanguageMetrics `json:"languages"` | ||
Editors []CopilotEditorMetrics `json:"editors"` | ||
} | ||
|
||
// CopilotIDEChat represents chat metrics in IDEs | ||
type CopilotIDEChat struct { | ||
TotalEngagedUsers int `json:"total_engaged_users"` | ||
Editors []CopilotEditorChatMetrics `json:"editors"` | ||
} | ||
|
||
// CopilotDotcomChat represents chat metrics on GitHub.com | ||
type CopilotDotcomChat struct { | ||
TotalEngagedUsers int `json:"total_engaged_users"` | ||
Models []CopilotChatModel `json:"models"` | ||
} | ||
|
||
// CopilotDotcomPullRequests represents pull request metrics on GitHub.com | ||
type CopilotDotcomPullRequests struct { | ||
TotalEngagedUsers int `json:"total_engaged_users"` | ||
Repositories []CopilotRepositoryMetrics `json:"repositories"` | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need all those to define. You can use restClient. Please check ListAlertsForRepo
in client/client.go
i.e.
metrics, resp, err := client.restClient.Copilot.GetOrganizationMetrics(ctx, owner, &googlegithub.CopilotMetricsListOptions{
Since: nil,
Until: nil,
ListOptions: googlegithub.ListOptions{
Page: 0,
PerPage: 0,
},
})
@austenstone how are you going with this? Do you need any assistance? I'd love to see this functionality added |
Hi! I've been slacking here. |
Introduces functionality to query and process GitHub Copilot metrics for organizations and teams. It includes new API integrations, data models, query handlers, and tests to support this feature.
Fixes #425