-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(dynamic-sampling): Settings for sample rate #79341
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
Merged
ArthurKnaus
merged 3 commits into
master
from
aknaus/feat/dynamic-sampling/settings-for-sample-rate
Oct 21, 2024
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
122 changes: 122 additions & 0 deletions
122
static/app/views/settings/dynamicSampling/dynamicSampling.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| import {css} from '@emotion/react'; | ||
| import styled from '@emotion/styled'; | ||
|
|
||
| import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator'; | ||
| import {Button} from 'sentry/components/button'; | ||
| import Confirm from 'sentry/components/confirm'; | ||
| import FieldGroup from 'sentry/components/forms/fieldGroup'; | ||
| import Panel from 'sentry/components/panels/panel'; | ||
| import PanelBody from 'sentry/components/panels/panelBody'; | ||
| import PanelHeader from 'sentry/components/panels/panelHeader'; | ||
| import {t} from 'sentry/locale'; | ||
| import {space} from 'sentry/styles/space'; | ||
| import {useMutation} from 'sentry/utils/queryClient'; | ||
| import useApi from 'sentry/utils/useApi'; | ||
| import useOrganization from 'sentry/utils/useOrganization'; | ||
| import {dynamicSamplingForm} from 'sentry/views/settings/dynamicSampling/dynamicSamplingForm'; | ||
| import {TargetSampleRateField} from 'sentry/views/settings/dynamicSampling/targetSampleRateField'; | ||
| import {useAccess} from 'sentry/views/settings/projectMetrics/access'; | ||
|
|
||
| const {useFormState, FormProvider} = dynamicSamplingForm; | ||
|
|
||
| export function DynamicSampling() { | ||
| const api = useApi(); | ||
| const organization = useOrganization(); | ||
| const {hasAccess} = useAccess({access: ['org:write']}); | ||
|
|
||
| const formState = useFormState({ | ||
| targetSampleRate: ((organization.targetSampleRate ?? 1) * 100)?.toLocaleString(), | ||
| samplingMode: 'auto' as const, | ||
| }); | ||
|
|
||
| const modeField = formState.fields.samplingMode; | ||
| const endpoint = `/organizations/${organization.slug}/`; | ||
|
|
||
| const {mutate: updateOrganization, isPending} = useMutation({ | ||
| mutationFn: () => { | ||
| const {fields} = formState; | ||
| return api.requestPromise(endpoint, { | ||
| method: 'PUT', | ||
| data: { | ||
| targetSampleRate: Number(fields.targetSampleRate.value) / 100, | ||
| }, | ||
| }); | ||
| }, | ||
| onSuccess: () => { | ||
| addSuccessMessage(t('Changes applied.')); | ||
| formState.save(); | ||
| }, | ||
| onError: () => { | ||
| addErrorMessage(t('Unable to save changes. Please try again.')); | ||
| }, | ||
| }); | ||
|
|
||
| const handleSubmit = () => { | ||
| updateOrganization(); | ||
| }; | ||
|
|
||
| const handleReset = () => { | ||
| formState.reset(); | ||
| }; | ||
|
|
||
| return ( | ||
| <FormProvider formState={formState}> | ||
| <form onSubmit={event => event.preventDefault()}> | ||
| <Panel> | ||
| <PanelHeader>{t('Automatic Sampling')}</PanelHeader> | ||
| <PanelBody> | ||
| <FieldGroup | ||
| label={t('Sampling Mode')} | ||
| help={t('Changes the level of detail and configuring sample rates.')} | ||
| > | ||
| {t('Automatic Balancing')} | ||
| </FieldGroup> | ||
| {/* TODO(aknaus): move into separate component when we make it interactive */} | ||
| <FieldGroup | ||
| disabled | ||
| label={t('Switch Mode')} | ||
| help={t( | ||
| 'Take control over the individual sample rates in your projects. This disables automatic adjustments.' | ||
| )} | ||
| > | ||
| <Confirm disabled> | ||
| <Button | ||
| title={t('This feature is not yet available.')} | ||
| css={css` | ||
| width: max-content; | ||
| `} | ||
| > | ||
| {modeField.value === 'auto' | ||
| ? t('Switch to Manual') | ||
| : t('Switch to Auto')} | ||
| </Button> | ||
| </Confirm> | ||
| </FieldGroup> | ||
| {modeField.value === 'auto' ? <TargetSampleRateField /> : null} | ||
| </PanelBody> | ||
| </Panel> | ||
| <FormActions> | ||
| <Button disabled={!formState.hasChanged || isPending} onClick={handleReset}> | ||
| {t('Reset')} | ||
| </Button> | ||
|
Comment on lines
+99
to
+101
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we add a tooltip informing the users why the button is disabled? It is pretty self-explanatory to me but imo it would be nice to have. |
||
| <Button | ||
| priority="primary" | ||
| disabled={ | ||
| !hasAccess || !formState.isValid || !formState.hasChanged || isPending | ||
| } | ||
| onClick={handleSubmit} | ||
| > | ||
| {t('Save changes')} | ||
| </Button> | ||
| </FormActions> | ||
| </form> | ||
| </FormProvider> | ||
| ); | ||
| } | ||
|
|
||
| const FormActions = styled('div')` | ||
| display: grid; | ||
| grid-template-columns: repeat(2, max-content); | ||
| gap: ${space(1)}; | ||
| justify-content: flex-end; | ||
| `; | ||
27 changes: 27 additions & 0 deletions
27
static/app/views/settings/dynamicSampling/dynamicSamplingForm.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import {t} from 'sentry/locale'; | ||
| import {createForm} from 'sentry/views/settings/dynamicSampling/formContext'; | ||
|
|
||
| type FormFields = { | ||
| samplingMode: 'auto' | 'manual'; | ||
| targetSampleRate: string; | ||
| }; | ||
|
|
||
| export const dynamicSamplingForm = createForm<FormFields>({ | ||
| validators: { | ||
| targetSampleRate: (value: string) => { | ||
| if (value === '') { | ||
| return t('This field is required.'); | ||
| } | ||
|
|
||
| const numericValue = Number(value); | ||
| if (isNaN(numericValue) ? t('Please enter a valid number.') : undefined) { | ||
| return t('Please enter a valid number.'); | ||
| } | ||
|
|
||
| if (numericValue < 0 || numericValue > 100) { | ||
| return t('The sample rate must be between 0% and 100%'); | ||
| } | ||
| return undefined; | ||
| }, | ||
| }, | ||
| }); |
147 changes: 147 additions & 0 deletions
147
static/app/views/settings/dynamicSampling/formContext.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| import {createContext, useCallback, useContext, useState} from 'react'; | ||
|
|
||
| interface FormState<FormFields extends Record<string, any>> { | ||
| /** | ||
| * State for each field in the form. | ||
| */ | ||
| fields: { | ||
| [K in keyof FormFields]: { | ||
| hasChanged: boolean; | ||
| initialValue: FormFields[K]; | ||
| onChange: (value: FormFields[K]) => void; | ||
| value: FormFields[K]; | ||
| error?: string; | ||
| }; | ||
| }; | ||
| /** | ||
| * Whether the form has changed from the initial values. | ||
| */ | ||
| hasChanged: boolean; | ||
| /** | ||
| * Whether the form is valid. | ||
| * A form is valid if all fields pass validation. | ||
| */ | ||
| isValid: boolean; | ||
| /** | ||
| * Resets the form state to the initial values. | ||
| */ | ||
| reset: () => void; | ||
| /** | ||
| * Saves the form state by setting the initial values to the current values. | ||
| */ | ||
| save: () => void; | ||
| } | ||
|
|
||
| export type FormValidators<FormFields extends Record<string, any>> = { | ||
| [K in keyof FormFields]?: (value: FormFields[K]) => string | undefined; | ||
| }; | ||
|
|
||
| type InitialValues<FormFields extends Record<string, any>> = { | ||
| [K in keyof FormFields]: FormFields[K]; | ||
| }; | ||
|
|
||
| /** | ||
| * Creates a form state object with fields and validation for a given set of form fields. | ||
| */ | ||
| export const useFormState = <FormFields extends Record<string, any>>(config: { | ||
| initialValues: InitialValues<FormFields>; | ||
| validators?: FormValidators<FormFields>; | ||
| }): FormState<FormFields> => { | ||
| const [initialValues, setInitialValues] = useState(config.initialValues); | ||
| const [values, setValues] = useState(initialValues); | ||
| const [errors, setErrors] = useState<{[K in keyof FormFields]?: string}>({}); | ||
|
|
||
| const setValue = useCallback( | ||
| <K extends keyof FormFields>(name: K, value: FormFields[K]) => { | ||
| setValues(old => ({...old, [name]: value})); | ||
| }, | ||
| [] | ||
| ); | ||
|
|
||
| const setError = useCallback( | ||
| <K extends keyof FormFields>(name: K, error: string | undefined) => { | ||
| setErrors(old => ({...old, [name]: error})); | ||
| }, | ||
| [] | ||
| ); | ||
|
|
||
| /** | ||
| * Validates a field by running the field's validator function. | ||
| */ | ||
| const validateField = useCallback( | ||
| <K extends keyof FormFields>(name: K, value: FormFields[K]) => { | ||
| const validator = config.validators?.[name]; | ||
| return validator?.(value); | ||
| }, | ||
| [config.validators] | ||
| ); | ||
|
|
||
| const handleFieldChange = <K extends keyof FormFields>( | ||
| name: K, | ||
| value: FormFields[K] | ||
| ) => { | ||
| setValue(name, value); | ||
| setError(name, validateField(name, value)); | ||
| }; | ||
|
|
||
| return { | ||
| fields: Object.entries(values).reduce((acc, [name, value]) => { | ||
| acc[name as keyof FormFields] = { | ||
| value, | ||
| onChange: inputValue => handleFieldChange(name as keyof FormFields, inputValue), | ||
| error: errors[name as keyof FormFields], | ||
| hasChanged: value !== initialValues[name], | ||
| initialValue: initialValues[name], | ||
| }; | ||
| return acc; | ||
| }, {} as any), | ||
| isValid: Object.values(errors).every(error => !error), | ||
| hasChanged: Object.entries(values).some( | ||
| ([name, value]) => value !== initialValues[name] | ||
| ), | ||
| save: () => { | ||
| setInitialValues(values); | ||
| }, | ||
| reset: () => { | ||
| setValues(initialValues); | ||
| setErrors({}); | ||
| }, | ||
| }; | ||
| }; | ||
|
|
||
| /** | ||
| * Creates a form context and hooks for a form with a given set of fields to enable type-safe form handling. | ||
| */ | ||
| export const createForm = <FormFields extends Record<string, any>>({ | ||
| validators, | ||
| }: { | ||
| validators?: FormValidators<FormFields>; | ||
| }) => { | ||
| const FormContext = createContext<FormState<FormFields> | undefined>(undefined); | ||
|
|
||
| function FormProvider({ | ||
| children, | ||
| formState, | ||
| }: { | ||
| children: React.ReactNode; | ||
| formState: FormState<FormFields>; | ||
| }) { | ||
| return <FormContext.Provider value={formState}>{children}</FormContext.Provider>; | ||
| } | ||
|
|
||
| const useFormField = <K extends keyof FormFields>(name: K) => { | ||
| const formState = useContext(FormContext); | ||
| if (!formState) { | ||
| throw new Error('useFormField must be used within a FormProvider'); | ||
| } | ||
|
|
||
| return formState.fields[name]; | ||
| }; | ||
|
|
||
| return { | ||
| useFormState: (initialValues: InitialValues<FormFields>) => | ||
| useFormState<FormFields>({initialValues, validators}), | ||
| FormProvider, | ||
| useFormField, | ||
| }; | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
67 changes: 67 additions & 0 deletions
67
static/app/views/settings/dynamicSampling/targetSampleRateField.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import styled from '@emotion/styled'; | ||
|
|
||
| import FieldGroup from 'sentry/components/forms/fieldGroup'; | ||
| import {InputGroup} from 'sentry/components/inputGroup'; | ||
| import {Tooltip} from 'sentry/components/tooltip'; | ||
| import {t} from 'sentry/locale'; | ||
| import {dynamicSamplingForm} from 'sentry/views/settings/dynamicSampling/dynamicSamplingForm'; | ||
| import {useAccess} from 'sentry/views/settings/projectMetrics/access'; | ||
|
|
||
| const {useFormField} = dynamicSamplingForm; | ||
|
|
||
| export function TargetSampleRateField({}) { | ||
| const field = useFormField('targetSampleRate'); | ||
| const {hasAccess} = useAccess({access: ['org:write']}); | ||
|
|
||
| return ( | ||
| <FieldGroup | ||
| disabled={!hasAccess} | ||
| required | ||
| label={t('Target Sample Rate')} | ||
| help={t( | ||
| 'Sentry will balance the sample rates of your projects automatically based on an overall target for your organization.' | ||
| )} | ||
| error={field.error} | ||
| > | ||
| <InputWrapper> | ||
| <Tooltip | ||
| disabled={hasAccess} | ||
| title={t('You do not have permission to change the sample rate.')} | ||
| > | ||
| <InputGroup> | ||
| <InputGroup.Input | ||
| width={100} | ||
| type="number" | ||
| disabled={!hasAccess} | ||
| value={field.value} | ||
| onChange={event => field.onChange(event.target.value)} | ||
| /> | ||
| <InputGroup.TrailingItems> | ||
| <TrailingPercent>%</TrailingPercent> | ||
| </InputGroup.TrailingItems> | ||
| </InputGroup> | ||
| </Tooltip> | ||
| {field.hasChanged ? ( | ||
| <PreviousValue>{t('previous rate: %f%%', field.initialValue)}</PreviousValue> | ||
| ) : null} | ||
| </InputWrapper> | ||
| </FieldGroup> | ||
| ); | ||
| } | ||
|
|
||
| const PreviousValue = styled('span')` | ||
| font-size: ${p => p.theme.fontSizeExtraSmall}; | ||
| color: ${p => p.theme.subText}; | ||
| `; | ||
|
|
||
| const InputWrapper = styled('div')` | ||
| padding-top: 8px; | ||
| height: 58px; | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: 4px; | ||
| `; | ||
|
|
||
| const TrailingPercent = styled('strong')` | ||
| padding: 0 2px; | ||
| `; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
just a thought: do we want to make sure this is of certain precision?