search

elastic / synthetics

Synthetic Monitoring with Real Browsers
MIT License
63 stars 38 forks source link

[Proposal] Separating monitor sources, monitors, and journeys #451

Closed lucasfcosta closed 2 years ago

lucasfcosta commented 2 years ago

Summary

We currently use the terms "journey" and "monitor" interchangeably to refer to different concepts, causing confusion among the team. The ambiguity of these terms then propagates to our DSL and UX, causing our users to be confused too.

To solve the aforementioned problems, this proposal defines the terms journey and monitor in terms of ontology, code, and UX.

This proposal is organised in the following way:

  1. It defines the terms Monitor Sources, Monitors, and Journeys, and explains how they relate to each other
  2. It explains how these definitions impact our Synthetics DSL
  3. It outlines further consequences to UX and documentation

Definitions

glossary_synth drawio(2)

☝️ I'd recommend readers to refer back to the entity relationship diagram as they read each concept.

⚠️ Please notice that the "monitor source" term is out-of-date in that diagram. We now use the term suite.

Core concepts

Related concepts

How these definitions solve the aforementioned problems

These definitions solve the aforementioned problems because they separate Kibana-related concepts, from code-related concepts.

The benefits of adopting such a distinction include:

As a perk, it also defines a "location", allowing us to have more clarity when referring to what instances of Heartbeat, the Elastic Agent, and the Synthetics Service, do: they run one or more monitors' journeys.

Changes to the Synthetics DSL

Considering the definitions above, these are the goals of the changes to the Synthetics DSL:

  1. Make the distinction between a journey and a monitor crystal clear.
  2. Allow users to control a monitor's behaviour from within the journey which will be associated with it

I've also considered we do not want a monitor to be able to have multiple journeys.

Adding a monitor property to the journey's callback arguments

Such a change would allow users to call methods from the object under monitor to override a monitor's settings.

Imagine, for example, a situation in which a user has an important journey which must run every 1 minute, but all their other journeys only need to run every ten minutes. Currently, to fulfil their needs, such a user would have to define two monitor sources, each of which has different execution interval settings.

After this proposal, the user could achieve the same outcome by calling monitor.setInterval.

// This journey uses the monitor source's interval settings
journey("check something not important", ({ monitor }) => {
  monitor.setId("my-usual-monitor");

  step("access page", async () => {
    await page.goto("https://www.example.com")
  });
});

// This journey overrides the monitor source's interval settings
journey("an important monitor", ({ monitor }) => {
  monitor.setId("my-important-monitor");
  monitor.setInterval("@every 1m");

  step("access page", async () => {
      await page.goto("https://www.elastic.co");
  });
})

☝️ I've chosen to use functions rather than property assignments so that users can't accidentally assign to a property that wasn't previously defined. That helps us prevent users being puzzled because they've made a typo.

As we move forward, we can add more methods to the monitor object, which allow users to override yet more settings, like, throttling parameters, for example.

Setting a monitor's id form within the journey

Currently, we either use the journey's name as the monitor's id, or, if a name is not defined, we use an id property defined by the user.

That id property is problematic because it leads users to think that a journey and a monitor are the same thing, when, in fact, they're not (as per the definitions above). Therefore, this proposal suggests adding a use method to the monitor object passed to the journey's callback. This method would set the id of the monitor to which the journey is bound.

// Before this proposal
journey({ id: "my-monitor" }, () => {
  step("my step", async () => {
    await page.goto("https://www.elastic.co")
  });
});

// After this proposal
journey("I can still name the journey", ({ monitor }) => {
  monitor.use({ id: "my-monitor" })

  step("my step", async () => {
    await page.goto("https://www.elastic.co")
  });
});

Impact to UX and Docs

Docs impact

Our docs must be carefully written so that they do not use the terms "monitor" and "journey" interchangeably.

It shouldn't be necessary for our docs to have such thorough definitions as I've written in this issue. Ideally, users should understand these concepts naturally, simply by reading the documentation. If they don't, it means we need to simplify even more.

Furthermore, it would be useful for our documentation to highlight that users can (and should) run their journeys in their machines, and CI, in addition to running them with their Elastic Stack. Highlighting such a benefit would help reinforcing the distinction between a journey and a monitor.

⭐ It's also worth highlighting that having excellent documentation isn't necessarily the most important factor when it comes to driving adoption. To drive adoption we should frequently reach users where they are through tutorials, workshops, blog posts, and talks. Cypress does a fantastic job at this, for example.

UX Impact

Our UI currently does not take into account the concepts above, and, therefore, it does not help in making those concepts clearer to users. Users should understand the concepts in this proposal intuitively, by simply by using the software, they shouldn't need to read definitions.

When users click the "Add Synthetics" button below after having gone through the integrations list, for example, it may lead them to think: "I already have this integration, why do I have to add it again so I can check more things?"

Screenshot 2022-02-04 at 17 04 32

Other places these definitions may impact significantly include

Open questions

Can we just call "journey" a "test"?

⭐ One thing that came to mind while writing this proposal is: why don't we just call a "journey" a "test"? Are we fighting against the term "test" because we want to define a new standard in the industry or for any other specific reason?

Given that users are already familiar with the term "test", wouldn't the distinction between a "journey" and a monitor be clearer if we used the term "test" to refer to a "journey"?

It feels to me like we may be fighting against a well-stablished definition here.

We also need to think carefully about that because that would be a significant change from what we and our users are used to. It would also have a profound impact in the UI, and docs.

⚠️ The answer to this question is no. We'll still use journey.

andrewvc commented 2 years ago

Great writeup @lucasfcosta I'll break up my reply to address different points made in this issue:

Syntax changes

  1. I like the proposed syntax changes. It definitely clarifies things. Should monitorId also become a function like other options? If it's not required I think we should just just make the "name" the only thing that goes in the map, everything else (including tags) would be a setter.
  2. In your example you have a page.goto not in a step. I assume that was a mistake, and I assume that these setters are not in step blocks, just confirming.
  3. I'd be OK renaming 'journey' to 'test'. In practice, I think we should just alias / deprecate journey and leave it that way for a long while. I don't see any urgency there. rspec lets users use either context or describe for the same thing, and that's OK IMHO, there's little harm in it.

The term observer

You mentioned maybe replacing location with observer. We do use the term observer in beats, and it comes there from ECS. I think location is actually preferable, and with the synthetics server users should no longer have to see the term 'observer'. The term location is more standard in the industry and less confusing IMHO.

Definitions of various terms

I love your clarification of terms, and think those should be in our docs. I'm having trouble determining if there's a delta between what you wrote and how we define the terms today. It looks to me like it all lines up with my current understanding of how our app works. Is that wrong? Did I miss something?

LMK if I missed anything else in your proposal that needs comment.

lucasfcosta commented 2 years ago

I'm glad the writeup has been useful, thanks @andrewvc 😊

With regards to your comments:

1. Should monitorId also become a function like other options?

I think it could. My concern was that it could cause confusion given that we use the journey's name as an id if one is not provided. Therefore, we would end-up with users being able to set an id in two places: as an argument (the name) or through a function (the id).

However, I think that actually making it a function allows it to be more discoverable given people will probably look at all they can do with what's passed to the callback. It also centralizes all the interaction with the monitor running the journey.

Happy to proceed that way. I've updated the issue to reflect the change.

  1. In your example you have a page.goto not in a step. I assume that was a mistake, and I assume that these setters are not in step blocks, just confirming.

That is correct, that was a mistake, thanks for catching that. Also updated those snippets.

  1. I'd be OK renaming 'journey' to 'test'. In practice, I think we should just alias / deprecate journey and leave it that way for a long while. I don't see any urgency there. rspec lets users use either context or describe for the same thing, and that's OK IMHO, there's little harm in it.

Great! I think renaming it would be great for our users as we'll be using a term with which they're already familiar.

  1. It looks to me like it all lines up with my current understanding of how our app works. Is that wrong? Did I miss something?

In terms of its inner workings I'd agree. I don't think there are any major differences between these concepts and how they map to code. However, there are small yet significant things to notice here:

  1. We use some of these terms interchangeably in our discussions, and that causes confusion. We should be more careful with the names we use. If you look at this comment for example, you'll see that the concepts seem deeply intertwined in the examples (at least to me).
  2. AFAIK, currently, an observer and location are not exactly the same thing. Even though they both determine where the monitors run, an observer is a piece of information used to enrich the result of a check, while a location is a separate entity and a concept which is used exclusively by the service (although that information can enrich results too). Therefore, I'd say there's an intersection between the two, but that they're not equal.
  3. Considering the terms outlined here, the term "monitor" is a misnomer in heartbeat. What heartbeat defines is a "monitor source".

Furthermore, one big concern I have is that these concepts are not surfaced throughout the UX/UI, and that's why it ends up being confusing for users. We should make sure that these terms become intuitive as a user interacts with the product, and that's what's not happening.

If you take @kyungeunni's question, for example, that's an instance where even someone from within the team ended up not knowing exactly the difference between a monitor and journey. Thus I believe that this lack of clarity will also be a problem for users who don't have such close contact with the product on a day-to-day basis.

kyungeunni commented 2 years ago

Thanks Lucas, this is a great write-up and very comprehensive!! I really like having terms and concepts defined somewhere so that we can reference them in the future, so I'm really grateful for putting all this thing up! great work! 🥇

A weird thing happened in my mind, it was clear while looking at the diagram and reading through the definition of terms and ontology. However, I got confused again when I reached the end of the comment and examples. I'll try to demonstrate what was my flow of thinking.

The diagram makes 💯 sense when looking at the state after monitors are created, or when setting up the inline browser monitor. But let's say I'm setting up browser monitors with _zipurl, things change.

  1. What I configure in the UI is the global setting for all the monitors.
  2. I must understand that each journey will create a monitor, I'm not creating a monitor and then assigning the journey test content.
  3. In fact, there is no journey term used when creating the inline monitor, so journey must mean to do something else
  4. Actually that is right, you need to wrap the code with the term journey so you let the system know that there are multiple monitors to be set up.
  5. Furthermore in your suggestions in section Adding a monitor property to the journey's callback arguments, I can see the monitor and journey is highly coupled, and the journey can configure the monitor(e.g. set the id or interval for the monitor). Even without considering the proposal, currently, if the journey is removed from the code, the monitor disappears(correct me if I'm wrong). Journey dictates monitor. Then journey equals monitor at this point in my mind.
  6. But when I go back to the diagram, the monitor is a higher concept that contains a journey and it makes sense in the perspective of how things work, and when you think about the case users just run the code in their machine.
  7. But again, if we introduce features setting up monitor configuration inside journey, would users understand it as a pure test code that they can run on their machine? What happens to the monitor object when they run it locally where there is no context of monitor? can we really decouple these two?

It is sorely how I think about things and my opinion though, it's probably just me being particularly confused and having a specific way of thinking about entities and relations between them. So in short, I'm not opposing anything but just wanted to share my view on it :)

justinkambic commented 2 years ago

TLDR; my comment focuses primarily on how this change could impact the Recorder downstream.

Firstly thanks to Lucas for kicking this off; the old maxim "months of work can save hours of planning" is true and discussions like this are best-effort attempts to avoid future difficulty.

I'm fine renaming journey to test as Andrew proposes if we feel that it will let us leverage the already well-understood concepts attached to that name. I've never had much difficulty distinguishing between these two terms or understanding their difference, but I've also lived with monitors much longer than journeys, so the concepts as Lucas has defined them makes total sense to me.

I think there's a tangent we might want to follow, perhaps as a breakout to avoid splintering the focus of this issue. I'll summarize. There are two main points I'd like to raise, both related to the recorder.

Terminology consistency for DX

Looking at this from the perspective of the recorder, we are using both the terms test and journey heavily and with distinct semantic differences in that codebase. This doesn't impact our users, so I think it's probably alright for now. However, in light of this proposal, and from a DX perspective, should we change this to avoid confusing engineers when they try to come to the codebase?

In the parlance of the Recorder, journey is the outputted code from the Synthetics library, exactly as defined by Lucas here. Test is used to refer to a run of that journey, to allow the user to ensure the script's results are reproducible and in accordance with their expectations. If we do replace journey with test, we're going to need to rename test-related concepts in the Recorder codebase to something else to avoid overloading that term and breaking specificity with the carefully-defined ontology proposed in this issue. The impact should be minimal in terms of work required.

Journey code generation

Playwright has no concept of what Synthetics does, so the code output from the Recorder is largely driven by interaction with PW utilities. We're not going to be generating any code using PW that will let our users leverage this new concept of monitor in Synthetics journey/test code.

We do have some rudimentary code addition utilities we maintain for creating inline vs. suite output. Presumably if we wanted to add monitor manipulation as a feature in the Recorder, we'd have to significantly expand this "overlay" style of code generation. I don't have an objection to this but wanted to point out potential downstream impact of this change on our other products if we did introduce this change to the Synthetics API.

liciavale commented 2 years ago

Hey, @lucasfcosta thank you for this, this is a very good call, I saw users struggling during the user testing because there was confusion about terminologies.

I think Journey makes sense to me, also because as Justin was pointing out Test to me means that we are testing the journey. We could replace Test with Replay, for example, but I think it makes more sense to test/replay a journey/user flow and not a test specifically.

We started a conversation also regarding the suites, not sure if we want to add this to this issue specifically or open a new one but we should come up with terminology that makes sense for the users also for this. Suites "creates" this entity that is ultimately a monitor but not sure users are familiar with this either Atm, I'm using "suites configuration" for the suites monitors creation flow but are we happy with this, or do we want to consider something else? Any suggestions?

lucasfcosta commented 2 years ago

Thank you all for your input, I'll answer you in a single comment to make this a bit easier 🙂

@kyungeunni that's excellent input, thanks a lot! It's very valuable to have a fresh pair of eyes looking at these concepts 🙌

It feels to me that the problem you describe is mostly related to the term "journey" itself. Is that an accurate assessment?

I thought so because if we renamed the DSL so that journey becomes test, for example, I would assume that a lot of that confusion would disappear, as test is a term with which developers are usually more familiar. Do you think that would help?

The reason why I thought it would be useful to have separate entities for journey and monitor is just so that we can talk clearly about code and something else which lives in the Elastic Stack. Furthermore, a journey will not necessarily yield a monitor. You can have multiple journeys feeding into a single monitor, for example.

It may also be worth highlighting that the term "inline" for browser monitors should eventually be hidden or change in some way. It is indeed confusing that the UI describes a monitor as "inline" when in fact you're creating a monitor source which has a journey's code inline. Thanks for highlighting this problem. I think that's something we could address in the UI. What do you think? Does this feel like a conceptual issue or an UI issue to you?

Please let me know if I correctly understood the points which were confusing to you 🙂

@justinkambic great points as always!

In terms of terminology consistency in the recorder, I think it would probably be useful to do another pass in the Figma designs and make sure that the copy there is consistent. The same goes for the code. Shall we add that to our catch-up agenda?

With regards to renaming occurrences of test in the recorder, I was thinking that we could call them "executions" or "test runs". Replay, as @liciavale suggested also sounds good, although I slightly prefer "test runs". "Test run" would also probably be more familiar for developers. Do you think that would be a good naming choice?

Finally, in terms of DX, I agree we should update our codebase (also for other projects). I also think that documenting these terms somewhere more permanent would be great. Maybe even our official docs (although we'd simplify it a bit to put it there). What do you think?

@liciavale thanks for the suggestions and for highlighting the problem with "suites".

Exactly as you describe "Suites "creates" this entity that is ultimately a monitor but not sure users are familiar with this either". Considering your description and the spec above, what we currently call a suite would be called a "monitor source".

I've described a "monitor source" as:

A monitor source is an entity which is capable of creating zero or more monitors. A monitor source may include an inline script, or a zip_url from which it will fetch a project containing multiple monitors.

Do you think that's accurate/easy to understand? Happy to update the definition if you think "suites" don't really map to a "monitor source".

Thank you all 🙌

andrewvc commented 2 years ago

I do think we are missing a verb/noun combo as others here have said. I like @lucasfcosta 's proposal for "test runs". It's the most intuitive IMHO.

liciavale commented 2 years ago

With regards to renaming occurrences of test in the recorder, I was thinking that we could call them "executions" or "test runs". Replay, as @liciavale suggested also sounds good, although I slightly prefer "test runs". "Test run" would also probably be more familiar for developers. Do you think that would be a good naming choice? Finally, in terms of DX, I agree we should update our codebase (also for other projects). I also think that documenting these terms somewhere more permanent would be great. Maybe even our official docs (although we'd simplify it a bit to put it there). What do you think?

@lucasfcosta sounds good to me, let's make sure we keep it consistent also in the monitor management flow - we have Run test atm once users uploaded the script before saving it.

@liciavale thanks for the suggestions and for highlighting the problem with "suites".

Exactly as you describe "Suites "creates" this entity that is ultimately a monitor but not sure users are familiar with this either". Considering your description and the spec above, what we currently call a suite would be called a "monitor source". I've described a "monitor source" as:

A monitor source is an entity which is capable of creating zero or more monitors. A monitor source may include an inline script, or a zip_url from which it will fetch a project containing multiple monitors.

Do you think that's accurate/easy to understand? Happy to update the definition if you think "suites" don't really map to a "monitor source".

How we are planning to handle this (it's still a wip so any suggestions are welcome) on the monitor management side, suites are like a kind of configuration, we were thinking to have a separate flow than the Add a monitor because users will need to configure this and they can also add many suites configurations for different monitors so we'll need a name to call this. We could say that is more similar to an integration.

kyungeunni commented 2 years ago

thanks @lucasfcosta!

You can have multiple journeys feeding into a single monitor

This is interesting, I didn't know about this! can you point me to where I can find more about this, please? This could potentially raise more questions if the rule for making a monitor is not clearly communicated with users.

With refined terminologies and UI aids, the confusion can be resolved from my perspective, so thanks for your explanation.

Although I still have a little of concern regarding monitor object being used within the journey in your proposal. If we want to separate these two entities and journey being just the test contents, do you think it's worth not exposing the monitor entity at all? I can see it can be useful for users to further refine each monitor, but I feel like it's inconsistent in different contexts(e.g. when running the code locally and running it in synthetics monitor context) and potentially break the separation that we'd like to achieve. What do you think?

vigneshshanmugam commented 2 years ago

Great writeup as always, Thanks for putting this together @lucasfcosta. Great points and discussion from everyone. I do agree with most of the take here like

Will list down broken by topics on what I find difficult to agree on

Overriding monitor in Synthetics agent

A monitor might contain more than one journey/test, We could restrict this once we change the way suites work and with discovery mode, A monitor source becomes directly mapped to a single journey. But having the control flow between Heartbeat and Synthetics feels a bit awkward for me.

I always felt HB is the API layer for synthetics agent when it comes to execution as it does most of the heavy lifting when it comes to Elastic stack and agent still being useful without using heartbeat and its terms (referring to monitor here). Having monitor tied to the journey would bring in lots of confusions for regular synthetic agent users.

I agree with @kyungeunni points shared above which expresses similar concerns. I believe the whole proposal started as a result of maintaining the monitor history and ensuring the id is intact. For the purpose of that and keeping the change simple, I think an alternative proposal would be to simply allow overriding the monitorId through the current journey context like you proposed before via monitorId instead of id or something like this below

journey("I can still name the journey", ({ page }) => {
  // applies only to current journey context
  journey.use({ monitorId: "hash" });

  step("my step", async () => { });
});

I would vouch any way to avoid bringing in monitor term to the synthetics agent and always keep it in the HB layer.

Journey as test

Maybe its just me, I always felt journey a bit superior than test, Test feels more like how an app should work while the journey dictates how a a user funnels through your application. In terms of the implementation detail, both feels similar.

But taking a step back and thinking about the future of Synthetics, we don't want to be a scripted testing platform instead a website performance measurement tool that can identify performance bottlenecks and give insights for an user flow/journey. Currently our focus is more on scripted e2e tests, I feel this would change once we step in to the other layer and journey feels lot more appropriate than test.

To me, I always felt our product alignment is more towards Lighthouse + Playwright than Cypress. One more reason journey is better than test is here - https://umaar.com/dev-tips/248-recorder-playback/

lucasfcosta commented 2 years ago

@liciavale thank you for taking the time to explain 🙌

With regards to this comment:

How we are planning to handle this (it's still a wip so any suggestions are welcome) on the monitor management side, suites are like a kind of configuration, we were thinking to have a separate flow than the Add a monitor because users will need to configure this and they can also add many suites configurations for different monitors so we'll need a name to call this. We could say that is more similar to an integration.

When you say this would be more similar to an integration, do you mean more similar to the entity that's created when you go to integrations and add a "synthetics integration", is that correct? If so, I'd say that this entity would be a "monitor source" as it's a piece of configuration which ends-up creating one or more monitors. What do you think? Would that be a concept we could map to a suite?

I'm not sure I fully understand what's the attribute/behaviour that a "suite" has that is currently missing from the "monitor source". Do you have thoughts on what would that be? Or is "monitor source" actually adequate here?

@kyungeunni, answering your questions:

This is interesting, I didn't know about this! can you point me to where I can find more about this, please? This could potentially raise more questions if the rule for making a monitor is not clearly communicated with users.

Unfortunately I don't think we have even documented that users can set an id for a journey. Currently it's a feature we have but that we haven't "shown" to users through public facing docs.

If you'd like to test it, however, you can currently create two different journeys with the same ID, as shown below:

journey({ id: "test-1", name: "something" }, () => {
  // Some code here.
});

journey({ id: "test-1", name: "something else" }, () => {
  // A different piece of code here.
});

Now, with regards to your other comments:

If we want to separate these two entities and journey being just the test contents, do you think it's worth not exposing the monitor entity at all?

That's interesting, so you'd say we should use the same term for the code which runs and for the entity in Kibana (and not call it a "monitor")? Or did I misunderstand?

My concern in that case (if I understand the suggestion correctly) is that a monitor is different from a journey in the sense that a monitor is stateful, while the journey is stateless.

I can see it can be useful for users to further refine each monitor, but I feel like it's inconsistent in different contexts(e.g. when running the code locally and running it in synthetics monitor context) and potentially break the separation that we'd like to achieve. What do you think?

I'm sorry, I'm not sure I fully understand this part of your comment. Is that related to not separating the monitor and journey concepts? Can you give me an example of what you envision in terms of what users would see/do? I think a practical example here would help me have a better understanding of your suggestion.

@vigneshshanmugam, thanks for the comments and for the detailed explanation. I'm glad the writeup was useful 🙌

I have a few questions/comments about what you've said:

Once we change the way suites work and with discovery mode, a monitor source becomes directly mapped to a single journey

For us to do that, wouldn't we have to deprecate zip_urls? I thought that a monitor source which uses a zip_url could yield multiple monitors (with multiple journeys). In that case, as long as we use zip_urls, wouldn't we have a 1:N mapping between monitor sources and monitors?

With regards to the syntax below, I really like it for setting a monitorId:

journey("I can still name the journey", ({ page }) => {
  // applies only to current journey context
  journey.use({ monitorId: "hash" });

  step("my step", async () => { });
});

My only question then becomes what would the API for setting a monitor's interval be in that case? Would we have an API like the one below?

journey("I can still name the journey", ({ page }) => {
  // applies only to current journey context
  journey.use({ monitorId: "hash", interval: "@every  3m" });

  step("my step", async () => { });
});

we don't want to be a scripted testing platform instead a website performance measurement tool that can identify performance bottlenecks and give insights for an user flow/journey

That's an excellent point! I hadn't thought about that 🤯

I agree with you that once we expand the term "test" could be constraining. Good catch.

Perhaps it would be good to hear @drewpost's thoughts on this too.

One alternative is to have both btw, like jest has it and test. Both have the same behaviour, but allow for people with different preferences when it comes to BDD/TDD to understand and use it.

paulb-elastic commented 2 years ago

Thanks for writing this up Lucas, and thanks everyone for the discussion - so useful for us to get this right, now.

A few comments / questions from me...

? Presumably we’d stub monitor (or some other solution) to ensure no problems calling monitor.setId() etc. in the journey and running it locally?

Monitor source

I do like this as it’s clear what it is. I wonder though, would users instinctively know what this is? For example, having an Add Monitor Source button? I’m probably overthinking this, as it’s clear exactly what it is, I’m just thinking of the simpler use case, where a user just wants to add a simple monitor for their homepage (something we’re going to simplify with an improved UX flow), they still need to think about this being a source (whereas they may just be thinking about it as a URL for their homepage). I’m not sure this can be reconciled (as we support a variety of configuration options), and I’m sure it will become easier to understand as they add a few monitors.

I already have this integration, why do I have to add it again so I can check more things?

Agreed, we’ve mentioned this elsewhere that it’s very confusing, and will be replaced as we’ll effectively deprecate this integration, in favour of one for local testing location (definitely not that name, but that concept).

Can we just call "journey" a "test"?

Do (our intended) users currently refer to what we deem a journey (i.e. the code), as a test? i.e. is a test more aligned to what users think of as a journey, as opposed to what they currently think of as a monitor? I know it is the case of some test frameworks (like Jest, although not others). I personally tend to think of test more in the context of a verb, and journey more aligns to the code for me, however I’ll definitely yield to the collective here..

...renaming occurrences of test in the recorder, I was thinking that we could call them "executions" or "test runs".

Assuming we change journey to test (from the above discussion), are we talking about calling them test runs when run from the script recorder and also when run locally (or via CI)? I assume the Uptime UI (or new UI) would still call them Monitor History (or something like that), as they are the results of the monitor running (for all types of monitor), in the context of the Elastic stack (i.e. not a test)?

Location or Observer: ...a location is a separate entity and a concept which is used exclusively by the service...

I don’t think of a location as a service specific thing. I personally think location will be more obvious to a user. I see a location is where the monitor runs and results collected from them (with a location property for the result). The location can be somewhere in the world where we host the service (e.g. US Central), could be somewhere I have Heartbeat running from (e.g. My basement), or could be where I have the Elastic Agent installed and configured through the new local testing location integration (not that name).

Considering the terms outlined here, the term "monitor" is a misnomer in heartbeat. What heartbeat defines is a "monitor source"

Are you suggesting we change heartbeat.monitors to heartbeat.monitors-source (or something similar)?

You can have multiple journeys feeding into a single monitor, for example.

How is that - I might be missing something here? I know you mention a case here, however, that’s not a use case we’re catering for are we, I’d expect that a bug (two different journeys pretending to be the same monitor)?

What happens to suites

I agree with (I think the sentiment above), that suites effectively go away don’t they, as the ZIP URL just becomes a property of that kind of monitor source. i.e. today we support a monitor source of a ZIP URL, but could also add some other kind of source in the future (e.g. GitHub directly, or an S3 bucket for example).

lucasfcosta commented 2 years ago

Hey @paulb-elastic, thanks for the comments 🙏

Here are my thoughts on those:

Presumably we’d stub monitor (or some other solution) to ensure no problems calling monitor.setId() etc. in the journey and running it locally?

Yes, that's correct! Purely from a software design point of view such a design also allows other monitor providers to hook into monitor, so it's really composable and concerns are nicely separated IMO 😄

Wonder though, would users instinctively know what this is? For example, having an Add Monitor Source button? I’m probably overthinking this, as it’s clear exactly what it is, I’m just thinking of the simpler use case, where a user just wants to add a simple monitor for their homepage (something we’re going to simplify with an https://github.com/elastic/uptime/issues/426), they still need to think about this being a source (whereas they may just be thinking about it as a URL for their homepage).

That's an excellent point! Indeed, we should try to avoid cognitive overload when they want to quickly add monitors. In this case, I think that we could solve it in the UI while still making the concept clear. We could, for example, have "Quick Add" button, or make sure that we hide/pre-fill some fields when adding a source. I think the important thing here is to make the flow quick and easy, but stick to the separation of concerns.

IMO even if we hide the term "monitor source", we should just go for an omission instead of trying to create another term.

Assuming we change journey to test (from the above discussion), are we talking about calling them test runs when run from the script recorder and also when run locally (or via CI)? I assume the Uptime UI (or new UI) would still call them Monitor History (or something like that), as they are the results of the monitor running (for all types of monitor), in the context of the Elastic stack (i.e. not a test)?

Good catch, I hadn't thought about that. I agree with you here. Given that a monitor is the Kibana-related entity, and that a journey (or test) is just a piece of code, I think "monitor history" (as it's called now) makes a lot of sense.

I don’t think of a location as a service specific thing. I personally think location will be more obvious to a user. I see a location is where the monitor runs and results collected from them (with a location property for the result). The location can be somewhere in the world where we host the service (e.g. US Central), could be somewhere I have Heartbeat running from (e.g. My basement), or could be where I have the Elastic Agent installed and configured through the new local testing location integration (not that name).

Exactly! Sorry, I think I wasn't clear in my previous comment. When I mentioned that a location and observer are currently different concepts (location being related to the service exclusively), I was talking about the current state of the stack. Currently, we use "locations" for the service, but "observers" within Heartbeat.

Going forward we'd use the same term for both as you suggested, so I totally agree with your point here.

Are you suggesting we change heartbeat.monitors to heartbeat.monitors-source (or something similar)?

Yes, I think we should be able to use heartbeat.monitor-source. Although I'd suggest that we don't deprecate monitors to avoid breaking changes for users who rely on this configuration key. As you mentioned earlier on the 1:1, I agree it's important that we're not disruptive with these changes, so I'd probably allow both keys to exist, but update documentation to point towards monitor-source.

How is that [referring to multiple journeys and a single monitor] - I might be missing something https://github.com/elastic/synthetics/issues/451#issuecomment-1034742834? I know you mention a case here, however, that’s not a use case we’re catering for are we, I’d expect that a bug (two different journeys pretending to be the same monitor)?

I'm fairly sure that you can currently have two different pieces of code feeding into the same monitor, especially considering that each Heartbeat instance has its own configuration file specifying the whole journey's inline code and the id of the monitor.

Or maybe I'm missing something and this would have other impacts which break things in Kibana or elsewhere in the stack? Is that a use-case we need to restrict? If so, how do we avoid that (especially for heartbeat.yml files with inline journeys)?

I agree with (I think the sentiment above), that suites effectively go away don’t they, as the ZIP URL just becomes a property of that kind of monitor source. i.e. today we support a monitor source of a ZIP URL, but could also add some other kind of source in the future (e.g. GitHub directly, or an S3 bucket for example).

Exactly. A monitor source could have different types, including inline, zip_url or s3, for example, and each of those types allows for it to have different attributes.

Almost like:

type BaseMonitorSource = { name: string, interval: string }

type InlineSource = BaseMonitorSource & { type: "inline", code: string }
type ZipURLSource = BaseMonitorSource & { type: "zip_url", url: string, username: string, password }

type BrowserMonitorSource = ZIPURLMonitorSource | InlineSource
lucasfcosta commented 2 years ago

As per our discussion in tech-sync, what we've decided is:

Also, we will have a separate breakout session to discuss:

lucasfcosta commented 2 years ago

Open questions:

Going forward

paulb-elastic commented 2 years ago

Cross linking to https://github.com/elastic/uptime/issues/458 as there's some potential cross over with name and/or id as referenced above (the ability to set these in the journey).

lucasfcosta commented 2 years ago

Here are our conclusions with regards to the DSL from the breakout session earlier today:

vigneshshanmugam commented 2 years ago

Closing this as we have agreed on a consensus and does not need further refinement.