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

Pyroscope: Rename phlare to grafana-pyroscope-datasource #68859

Merged
merged 26 commits into from
Jun 7, 2023

Conversation

ryantxu
Copy link
Member

@ryantxu ryantxu commented May 22, 2023

This PR renames the builtin "phlare" datasource to "grafana-pyroscope-datasource" and adds registers an alias in the plugin registry. This will keep existing settings working while allowing us to rebrand the plugin.

Depends on #67867, starts to address #17186

Alternative to:
#67847
#67490

@ryantxu ryantxu added type/chore no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels May 22, 2023
@ryantxu ryantxu added this to the 10.1.x milestone May 22, 2023
@ryantxu ryantxu requested review from tolzhabayev and a team as code owners May 22, 2023 22:39
@ryantxu ryantxu requested review from aocenas and removed request for a team, leventebalogh, marefr, papagian, suntala, L-M-K-B and eledobleefe June 2, 2023 18:54
@ryantxu ryantxu added add to changelog and removed no-changelog Skip including change in changelog/release notes labels Jun 2, 2023
@ryantxu ryantxu changed the title Chore: Rename phlare to grafana-pyroscope-datasource Pyroscope: Rename phlare to grafana-pyroscope-datasource Jun 2, 2023
@ryantxu ryantxu added the datasource/grafana-pyroscope Grafana pyroscope datasource (previously Phlare) label Jun 2, 2023
@ryantxu
Copy link
Member Author

ryantxu commented Jun 5, 2023

@aocenas -- i think this is good to go, but will wait for your ✅ before merging :)

Copy link
Member

@wbrowne wbrowne left a comment

Choose a reason for hiding this comment

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

LGTM other than the switch

@@ -165,6 +166,10 @@ func ReadPluginJSON(reader io.Reader) (JSONData, error) {
switch plugin.ID {
case "grafana-piechart-panel":
plugin.Name = "Pie Chart (old)"
case "grafana-pyroscope": // rebranding
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
case "grafana-pyroscope": // rebranding
case "grafana-pyroscope-datasource": // rebranding

and subsequently the Phlare<>Pyroscope rebranding -- hardcoded alias test

@aocenas
Copy link
Member

aocenas commented Jun 6, 2023

Looks like this works, don't really understand the code well enough to comment on that but seems like there was discussion enough about that already. Just wondering why grafana-pyroscope-datasource and not just grafana-pyroscope?

@ryantxu
Copy link
Member Author

ryantxu commented Jun 7, 2023

Just wondering why grafana-pyroscope-datasource and not just grafana-pyroscope?

all normal plugins end with their plugin type... typically {org}-{name}-{type}

@ryantxu ryantxu enabled auto-merge (squash) June 7, 2023 02:57
@ryantxu ryantxu merged commit e17ef5e into main Jun 7, 2023
7 checks passed
@ryantxu ryantxu deleted the phlare-to-pyroscope-ds branch June 7, 2023 03:09
@ricky-undeadcoders ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants