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

ref(releasehealth): Implement get_project_sessions_count on metrics backend [INGEST-249] #29148

Merged
merged 4 commits into from
Oct 11, 2021

Conversation

RaduW
Copy link
Contributor

@RaduW RaduW commented Oct 7, 2021

Like in #28598, port get_project_sessions_count from sentry.snuba.sessions to metrics.

INGEST-249

@RaduW RaduW requested review from jjbayer, untitaker and iProgramStuff and removed request for jjbayer October 7, 2021 10:55
Copy link
Member

@jjbayer jjbayer left a comment

Choose a reason for hiding this comment

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

Looks good! Feel free to ignore the nits.


query = Query(
dataset=Dataset.Metrics.value,
match=Entity("metrics_counters"),
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think we have the EntityKey enum for this now, we might as well use it.

@@ -90,6 +91,21 @@ def filter_releases_by_project_release(
)


def _model_environment_ids_to_environment_names(
Copy link
Member

Choose a reason for hiding this comment

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

nit: Will this function be used anywhere else? If not, I wonder if we could simplify to

def _get_environment_name(environment_id: int) -> Optional[str]:
    try:
        return Environment.objects.get(id=environment_id).name or None
     except Environment.DoesNotExist:
        return None

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, I thought about just getting one environment and then changed my mind... maybe will inline when we are done.

@@ -90,6 +91,21 @@ def filter_releases_by_project_release(
)


def _model_environment_ids_to_environment_names(
metric_ids: Sequence[int],
Copy link
Member

Choose a reason for hiding this comment

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

Should we call this environment_ids instead of metric_ids?

@RaduW RaduW merged commit 363f460 into master Oct 11, 2021
@RaduW RaduW deleted the feat/get_project_sessions_count branch October 11, 2021 08:27
@github-actions github-actions bot locked and limited conversation to collaborators Oct 26, 2021
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.

4 participants