Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Change subscope to be named Datacatalog #4

Merged
merged 1 commit into from
Sep 16, 2019
Merged

Conversation

chanadian
Copy link
Contributor

@chanadian chanadian commented Sep 16, 2019

Change the subscope of the metrics to be datacatalog instead of generic service. This is because the top level metricScope can be namespaced anything. So we should allow clients to use a metricScope that does not have to dictate datacatalog.

@chanadian chanadian changed the title Change subscope to be named datacatalog Change subscope to be named Datacatalog Sep 16, 2019
@chanadian chanadian requested a review from EngHabu September 16, 2019 20:49
@@ -68,7 +68,7 @@ func (s *DataCatalogService) AddTag(ctx context.Context, request *catalog.AddTag
func NewDataCatalogService() *DataCatalogService {
configProvider := runtime.NewConfigurationProvider()
dataCatalogConfig := configProvider.ApplicationConfiguration().GetDataCatalogConfig()
catalogScope := promutils.NewScope(dataCatalogConfig.MetricsScope).NewSubScope("service")
catalogScope := promutils.NewScope(dataCatalogConfig.MetricsScope).NewSubScope("datacatalog")
Copy link
Contributor

Choose a reason for hiding this comment

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

so it'll say flyte:datacatalog ? not datacatalog:something ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the first level can be flyte if that's what you set in the config.yaml.

@chanadian chanadian merged commit 2ee1d75 into master Sep 16, 2019
@chanadian chanadian deleted the prometheus-namespace branch September 16, 2019 21:36
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
Change subscope to be named Datacatalog
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants