Skip to content

Commit

Permalink
chore: added new relic telemetry for spans (#34240)
Browse files Browse the repository at this point in the history
## Description
> This PR adds telemetry and logs for different spans where tenant info
is fetched.

Fixes #34072  

## Automation

/ok-to-test tags="@tag.Sanity"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/9511509976>
> Commit: d4dc99e
> Cypress dashboard url: <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9511509976&attempt=1"
target="_blank">Click here!</a>

<!-- end of auto-generated comment: Cypress test results  -->






## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Enhanced tenant fetching with improved logging and metrics using
Micrometer.

- **Improvements**
- Added monitoring capabilities to various tenant-related services for
better observability and debugging.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
NilanshBansal authored Jun 14, 2024
1 parent 230c225 commit b4ca723
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.appsmith.external.constants.spans;

import static com.appsmith.external.constants.spans.BaseSpan.APPSMITH_SPAN_PREFIX;

public class TenantSpan {
public static final String FETCH_DEFAULT_TENANT_SPAN = APPSMITH_SPAN_PREFIX + "fetch_default_tenant";
public static final String FETCH_TENANT_CACHE_POST_DESERIALIZATION_ERROR_SPAN =
APPSMITH_SPAN_PREFIX + "fetch_tenant_cache_post_deserialization_error";
public static final String FETCH_TENANT_FROM_DB_SPAN = APPSMITH_SPAN_PREFIX + "fetch_tenant_from_db";
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.appsmith.server.helpers.InMemoryCacheableRepositoryHelper;
import com.appsmith.server.repositories.ce_compatible.CacheableRepositoryHelperCECompatibleImpl;
import io.micrometer.observation.ObservationRegistry;
import org.springframework.data.mongodb.core.ReactiveMongoOperations;
import org.springframework.stereotype.Component;

Expand All @@ -11,7 +12,8 @@ public class CacheableRepositoryHelperImpl extends CacheableRepositoryHelperCECo

public CacheableRepositoryHelperImpl(
ReactiveMongoOperations mongoOperations,
InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper) {
super(mongoOperations, inMemoryCacheableRepositoryHelper);
InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper,
ObservationRegistry observationRegistry) {
super(mongoOperations, inMemoryCacheableRepositoryHelper, observationRegistry);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,19 @@
import com.appsmith.server.helpers.InMemoryCacheableRepositoryHelper;
import com.appsmith.server.helpers.ce.bridge.Bridge;
import com.appsmith.server.helpers.ce.bridge.BridgeQuery;
import io.micrometer.observation.ObservationRegistry;
import lombok.extern.slf4j.Slf4j;
import net.minidev.json.JSONObject;
import org.springframework.data.mongodb.core.ReactiveMongoOperations;
import org.springframework.data.mongodb.core.query.Query;
import org.springframework.stereotype.Component;
import reactor.core.observability.micrometer.Micrometer;
import reactor.core.publisher.Mono;

import java.util.Set;
import java.util.stream.Collectors;

import static com.appsmith.external.constants.spans.TenantSpan.FETCH_TENANT_FROM_DB_SPAN;
import static com.appsmith.server.constants.FieldName.PERMISSION_GROUP_ID;
import static com.appsmith.server.constants.ce.FieldNameCE.ANONYMOUS_USER;
import static com.appsmith.server.constants.ce.FieldNameCE.DEFAULT_PERMISSION_GROUP;
Expand All @@ -35,12 +38,15 @@
public class CacheableRepositoryHelperCEImpl implements CacheableRepositoryHelperCE {
private final ReactiveMongoOperations mongoOperations;
private final InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper;
private final ObservationRegistry observationRegistry;

public CacheableRepositoryHelperCEImpl(
ReactiveMongoOperations mongoOperations,
InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper) {
InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper,
ObservationRegistry observationRegistry) {
this.mongoOperations = mongoOperations;
this.inMemoryCacheableRepositoryHelper = inMemoryCacheableRepositoryHelper;
this.observationRegistry = observationRegistry;
}

@Cache(cacheName = "permissionGroupsForUser", key = "{#user.email + #user.tenantId}")
Expand Down Expand Up @@ -182,13 +188,17 @@ public Mono<Tenant> fetchDefaultTenant(String tenantId) {
BridgeQuery<Tenant> andCriteria = Bridge.and(defaultTenantCriteria, notDeletedCriteria);
Query query = new Query();
query.addCriteria(andCriteria);

return mongoOperations.findOne(query, Tenant.class).map(tenant -> {
if (tenant.getTenantConfiguration() == null) {
tenant.setTenantConfiguration(new TenantConfiguration());
}
return tenant;
});
log.info("FETCHING TENANT FROM DATABASE AS NOT PRESENT IN CACHE!");
return mongoOperations
.findOne(query, Tenant.class)
.map(tenant -> {
if (tenant.getTenantConfiguration() == null) {
tenant.setTenantConfiguration(new TenantConfiguration());
}
return tenant;
})
.name(FETCH_TENANT_FROM_DB_SPAN)
.tap(Micrometer.observation(observationRegistry));
}

@CacheEvict(cacheName = "tenant", key = "{#tenantId}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.appsmith.server.helpers.InMemoryCacheableRepositoryHelper;
import com.appsmith.server.repositories.ce.CacheableRepositoryHelperCEImpl;
import io.micrometer.observation.ObservationRegistry;
import lombok.extern.slf4j.Slf4j;
import org.springframework.data.mongodb.core.ReactiveMongoOperations;
import org.springframework.stereotype.Component;
Expand All @@ -12,7 +13,8 @@ public class CacheableRepositoryHelperCECompatibleImpl extends CacheableReposito
implements CacheableRepositoryHelperCECompatible {
public CacheableRepositoryHelperCECompatibleImpl(
ReactiveMongoOperations mongoOperations,
InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper) {
super(mongoOperations, inMemoryCacheableRepositoryHelper);
InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper,
ObservationRegistry observationRegistry) {
super(mongoOperations, inMemoryCacheableRepositoryHelper, observationRegistry);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.appsmith.server.repositories.TenantRepository;
import com.appsmith.server.services.ce.TenantServiceCEImpl;
import com.appsmith.server.solutions.EnvManager;
import io.micrometer.observation.ObservationRegistry;
import jakarta.validation.Validator;
import org.springframework.context.annotation.Lazy;
import org.springframework.stereotype.Service;
Expand All @@ -23,7 +24,8 @@ public TenantServiceImpl(
@Lazy EnvManager envManager,
FeatureFlagMigrationHelper featureFlagMigrationHelper,
CacheableRepositoryHelper cacheableRepositoryHelper,
CommonConfig commonConfig) {
CommonConfig commonConfig,
ObservationRegistry observationRegistry) {
super(
validator,
repository,
Expand All @@ -32,6 +34,7 @@ public TenantServiceImpl(
envManager,
featureFlagMigrationHelper,
cacheableRepositoryHelper,
commonConfig);
commonConfig,
observationRegistry);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,18 @@
import com.appsmith.server.services.BaseService;
import com.appsmith.server.services.ConfigService;
import com.appsmith.server.solutions.EnvManager;
import io.micrometer.observation.ObservationRegistry;
import jakarta.validation.Validator;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.annotation.Lazy;
import org.springframework.util.StringUtils;
import reactor.core.observability.micrometer.Micrometer;
import reactor.core.publisher.Mono;

import java.util.Map;

import static com.appsmith.external.constants.spans.TenantSpan.FETCH_DEFAULT_TENANT_SPAN;
import static com.appsmith.external.constants.spans.TenantSpan.FETCH_TENANT_CACHE_POST_DESERIALIZATION_ERROR_SPAN;
import static com.appsmith.server.acl.AclPermission.MANAGE_TENANT;
import static java.lang.Boolean.TRUE;

Expand All @@ -44,6 +48,7 @@ public class TenantServiceCEImpl extends BaseService<TenantRepository, Tenant, S
private final CacheableRepositoryHelper cacheableRepositoryHelper;

private final CommonConfig commonConfig;
private final ObservationRegistry observationRegistry;

public TenantServiceCEImpl(
Validator validator,
Expand All @@ -53,13 +58,15 @@ public TenantServiceCEImpl(
@Lazy EnvManager envManager,
FeatureFlagMigrationHelper featureFlagMigrationHelper,
CacheableRepositoryHelper cacheableRepositoryHelper,
CommonConfig commonConfig) {
CommonConfig commonConfig,
ObservationRegistry observationRegistry) {
super(validator, repository, analyticsService);
this.configService = configService;
this.envManager = envManager;
this.featureFlagMigrationHelper = featureFlagMigrationHelper;
this.cacheableRepositoryHelper = cacheableRepositoryHelper;
this.commonConfig = commonConfig;
this.observationRegistry = observationRegistry;
}

@Override
Expand Down Expand Up @@ -175,6 +182,8 @@ public Mono<Tenant> getDefaultTenant() {
// Fetching Tenant from redis cache
return getDefaultTenantId()
.flatMap(tenantId -> cacheableRepositoryHelper.fetchDefaultTenant(tenantId))
.name(FETCH_DEFAULT_TENANT_SPAN)
.tap(Micrometer.observation(observationRegistry))
.flatMap(tenant -> repository.setUserPermissionsInObject(tenant).switchIfEmpty(Mono.just(tenant)))
.onErrorResume(e -> {
log.error("Error fetching default tenant from redis!", e);
Expand All @@ -191,6 +200,8 @@ public Mono<Tenant> getDefaultTenant() {
}
return tenant;
}))
.name(FETCH_TENANT_CACHE_POST_DESERIALIZATION_ERROR_SPAN)
.tap(Micrometer.observation(observationRegistry))
.flatMap(tenant -> repository
.setUserPermissionsInObject(tenant)
.switchIfEmpty(Mono.just(tenant)));
Expand Down

0 comments on commit b4ca723

Please sign in to comment.