Define service boundaries within the monolith

[markstory]

Building on the ideas presented in #2, this RFC aims to provide a more formal description of how services/domains within the monolith could be structured.

Rendered RFC

[wedamija]

Please no co-located tests, I think mirrored paths works well and keeps day to day work uncluttered

[mitsuhiko]

Yeah I would rather also see tests sit next to the package they go into, not inside the module structure. That said, I noticed the frontend has co-located tests now so I guess we are going down that path somewhere already.

[markstory]

Yeah I would rather also see tests sit next to the package they go into, not inside the module structure.

What does this look like for you? I'll update the RFC to reflect a separate directory tree for tests as we have today.

[bukzor]

I'm unsure how this line relates to the current draft. I don't see that co-located tests are recommended, above. Unless you're calling this directory structure co-location, but this isn't the first thing people think of when they hear "co-located tests".

src/sentry/discover
    models/discoversavedquery.py
    tests/models/test_discoversavedquery.py

Test co-location usually means (something equivalent to):

src/sentry/discover
    models/discoversavedquery.py
    models/discoversavedquery_test.py

I personally like that model, but as you note it's not commonplace in python.

[mitsuhiko]

I would propose to separate out the colocation discussion about tests into a separate proposal and to remove it form the scope of this. I think the test location discussion has a high bikeshedding potential but I think underpinning that largely philosophical debate are good reasons for one or the other which deserve to be discussed independently.

Either way I agree that we need a proposal for where tests go, but that we can have independently of the service boundaries.

[markstory]

I'm unsure how this line relates to the current draft. I don't see that co-located tests are recommended, above.

This is a vestigial change from a previous rev. I'll take it out.

I would propose to separate out the colocation discussion about tests into a separate proposal and to remove it form the scope of this.

Agree that this could be a big bikeshed.

[bukzor]

There must be no "common" or "utils" or "misc" directory, or you'll eventually get a fresh new cobweb of code with unclear ownership. Under the current proposal, as written, each (kind of, or owner of) shared code must be given a name and a directory at src/sentry/*.

to your examples

• src/sentry/rate-limiting
• src/sentry/project/models/project.py
• src/sentry/organization/models/organization.py

It's tempting to combine project and organization into a single "core" or "sentry" service, but then there's no clarity as to what should be added to (or rejected from) that service. Keep in mind that the "project" service will not contain just the one file but all the endpoints, serializers, tasks and tests that are specific to "project". And any proposed additions to this service will have a very clear admission criteria: is it actually about "project" or does it have some other scope.

But that's just my two cents :)

[markstory]

There must be no "common" or "utils" or "misc" directory, or you'll eventually get a fresh new cobweb of code with unclear ownership.

💯 agree that there should be no utils service.

It's tempting to combine project and organization into a single "core" or "sentry" service, but then there's no clarity as to what should be added to (or rejected from) that service.

Yes, we would need really clear boundaries on what goes into the 'core' service should it be created. Defining what the services are and where the lines in the sand should be should be a separate discussion in my opinion.

[bukzor]

The point that I tried to make was that "core" by definition has no clear boundary. Even if you make a document about its boundary, that doc is liable to be "improved" or ignored. Whereas a "project" service does have clear boundary by definition.

Note: I'm just trying to be extra clear. I don't actually have a strong opinion on this.

[markstory]

Note: I'm just trying to be extra clear. I don't actually have a strong opinion on this.

Thank you for the clarification. I agree that 'core' will be magnet of increasing scope. 'projects' could be a good container for organization, project and related settings that those resources have.

[fpacifici]

I think we should try to prevent the code of a service to directly access models and tables from another service by doing joins across service boundaries. Joining DB tables across modules would defy the idea of creating boundaries.
When referencing the module class this is not different than preventing a service from accessing the internals of another one, though with Django you can run raw SQL queries. Do we allow that to begin with, and if yes, should we prevent it ?

[markstory]

I think we should try to prevent the code of a service to directly access models and tables from another service by doing joins across service boundaries. Joining DB tables across modules would defy the idea of creating boundaries.

I agree. I considered recommending not exporting Django models from a service. The problem came up against though was that python's typehints will want an importable class to use in typehints, which would generally need to be the model 😢