-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Added instance domain to telemetry #2310
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -94,6 +94,7 @@ jobs: | |||
ENABLE_PAID_ENTERPRISE_EDITION_FEATURES=true \ | |||
AUTH_TYPE=basic \ | |||
REQUIRE_EMAIL_VERIFICATION=false \ |
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.
this should disable telemetry spam from our integration tests
@@ -57,6 +57,7 @@ | |||
KV_GEN_AI_KEY_CHECK_TIME = "genai_api_key_last_check_time" | |||
KV_SETTINGS_KEY = "danswer_settings" | |||
KV_CUSTOMER_UUID_KEY = "customer_uuid" |
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.
the new value is stored in postgres as a kv so it stays consistent once generated
@@ -21,14 +28,43 @@ class RecordType(str, Enum): | |||
FAILURE = "failure" | |||
|
|||
|
|||
def get_or_generate_uuid() -> str: | |||
def _get_or_generate_uuid() -> str: | |||
global _CACHED_UUID |
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.
added caching for uuid generation
kv_store.store(KV_CUSTOMER_UUID_KEY, customer_id, encrypt=True) | ||
return customer_id | ||
with Session(get_sqlalchemy_engine()) as db_session: | ||
first_user = db_session.query(User).first() |
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.
The logic to determine the "domain" of the danswer instance is to get the first user and use everything after the @ symbol of their email (E.g. danswer.ai)
payload = { | ||
"data": data, | ||
"record": record_type, | ||
# If None then it's a flow that doesn't include a user | ||
# For cases where the User itself is None, a string is provided instead | ||
"user_id": user_id, | ||
"customer_uuid": get_or_generate_uuid(), | ||
"customer_uuid": customer_uuid, | ||
} |
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.
only run if EE is enabled (and Telemetry isnt disabled)
backend/danswer/utils/telemetry.py
Outdated
@@ -21,14 +28,43 @@ class RecordType(str, Enum): | |||
FAILURE = "failure" | |||
|
|||
|
|||
def get_or_generate_uuid() -> str: | |||
def _get_or_generate_uuid() -> str: |
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.
can we generate this on startup?
@@ -324,6 +325,8 @@ async def lifespan(app: FastAPI) -> AsyncGenerator: | |||
# fill up Postgres connection pools | |||
await warm_up_connections() | |||
|
|||
get_or_generate_uuid() |
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.
can we add a comment here about why we need this?
Description
This pr adds a field called instance_domain to the anonymous_telemetry. This is only sent if EE features are enabled and telemetry is also enabled. the purpose is to help us track usage of our enterprise users per org (not per user)