The i18n strings were getting long and starting to be a pain on the eyes. I played around with some different formats and I'm pretty pleased with this. Not saying we should do it everywhere but I think it's worth experimenting with. A couple of goals:

• the resulting call t('key', 'label') should be as short and concise as possible
• when renaming a file it should be easy to rename all i18n strings simultaneously
• the wrapper should not be too abstracted away from the original

I'm assuming this breaks the i18n check?

Maybe we can stop using a hierarchy for our labels. Instead of using xpack.apm.settings.agentConfig.flyout.serviceSection.deleteConfigSucceededTitle, we can use xpack.apm.deleteAgentConfigSucceededTitle?

I'm assuming this breaks the i18n check?

Hmm.. I do get this error when running the i18n check:

   ✖ x-pack/legacy/plugins/apm
     → EISDIR: illegal operation on a directory, read

Any idea how to fix?

Maybe we can stop using a hierarchy for our labels. Instead of using xpack.apm.settings.agentConfig.flyout.serviceSection.deleteConfigSucceededTitle, we can use xpack.apm.deleteAgentConfigSucceededTitle

That would still be something like:

i18n.translate('xpack.apm.deleteAgentConfigSucceededTitle', {
  defaultMessage: 'Delete configuration'
}),

vs

t('deleteAgentConfigSucceededTitle', 'Delete configuration'),

Ofc if we can't make the later work then there's no way around it, but after having played around with it, it's a joy to have strings as one-liners again, and my eyes slowly start reading the code again (instead of just ignoring all the stuff my brain perceives as noise)

I'm not sure it is the best approach to group the values like this. I could also drop the grouping:

<SettingsSection
  // sample rate
  sampleRate={sampleRate}
  setSampleRate={setSampleRate}
  isSampleRateValid={isSampleRateValid}

  // capture body  
  captureBody={captureBody}
  setCaptureBody={setCaptureBody}

  // transaction max spans
  transactionMaxSpans={transactionMaxSpans}
  setTransactionMaxSpans={setTransactionMaxSpans}
  isTransactionMaxSpansValid={isTransactionMaxSpansValid}
/>;

Note to self: fix

just curious, why parseFloat? because it's more permissive than Number?

The opposite actually. Due to coercion Number('') => 0 whereas parseFloat('') => NaN.
I wanted to disallow empty values.

I'm assuming this breaks the i18n check?

What's the value of this over just using AgentConfiguration directly?

I wanted to make sure the method returned an optional (x | undefined). The full ES document is returned not just AgentConfiguration. I was looking for something like ESSearchHit<AgentConfiguration> but didn't find anything.

Oh, I get it now. Maybe we can expose something like that as well in our ES typings.

Good idea. I can add it. ESSearchHit good with you or any suggestions?

Hmm, I guess this is kind of where "existingEnvironments" breaks down. Maybe unconfigured or available is better here.

I found available to be misleading so changed it. But perhaps it's not objectively a good change.
I think unconfigured works 👍

should this not be environment? I might have mixed up environment from the configuration and (the available) environments from the UI.

Yes, a configuration is only linked to a single environment.

Oh sorry! I thought I changed this to environment already so when I saw your comment I misunderstood. Must have undone my changes. Will make it environment (and not environments)

considering that this data could change, should we even cache it?

Hmm... I thought about it but I think it's okay since it's only in-memory caching, so whenever the user comes back to Kibana they will see fresh data. It is also not data that changes very often. Most users have long running application names. Do you think it's likely that someone changes the service name or adds a new service while being on this page?

Hmm, maybe. I can imagine they want to configure a service around the time it starts sending data. Then again, they will probably just refresh the page anyway.