Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
a961238
Simplify integration and enable feature-flag support
mujacica Nov 19, 2025
eecf68c
Fix typing
mujacica Nov 19, 2025
d456eeb
feat(perforce): Add backend support for Perforce integration
mujacica Nov 11, 2025
cc6558b
Added with frontend
mujacica Nov 13, 2025
7a2499a
Code cleanup
mujacica Nov 13, 2025
eaac406
Fix pr comments and tests
mujacica Nov 13, 2025
54f4d1a
Fix tests
mujacica Nov 13, 2025
4b42f43
Fix PR reviews and tests
mujacica Nov 13, 2025
b042b64
Fix commit info
mujacica Nov 13, 2025
8c70da0
Remove P4Web (deprecated) and fix paths for swarm
mujacica Nov 18, 2025
860917b
Implement comments on the SSL/P4Port
mujacica Nov 18, 2025
0300c1d
Fix PR comments
mujacica Nov 18, 2025
2564313
Parse file revision properly
mujacica Nov 19, 2025
655269d
Fix PR Comments
mujacica Nov 19, 2025
46d9f3b
Simplify for installation only
mujacica Nov 19, 2025
36dda1a
Rework based on the PR comments
mujacica Nov 20, 2025
936db40
More cursor comment fixes
mujacica Nov 20, 2025
c231f85
Fix ticket authentication
mujacica Nov 20, 2025
d7438bf
Fix trust order of operations
mujacica Nov 20, 2025
6a372ed
Fix auth type issues
mujacica Nov 20, 2025
979d75d
Restore deleted logo
mujacica Nov 24, 2025
b8659e3
Fix stale config after update
mujacica Nov 24, 2025
d034c99
Fix cursor comment
mujacica Nov 24, 2025
990396a
Review comments fixes
mujacica Dec 1, 2025
f3a0a75
Simplify integration and enable feature-flag support
mujacica Nov 19, 2025
8889198
Fix typing
mujacica Nov 19, 2025
4aa759b
feat(perforce): Add backend support for Perforce integration
mujacica Nov 11, 2025
374a83d
feat(perforce): Implement repository/depot and code mapping logic
mujacica Nov 19, 2025
2b6ca4c
Fixes after rebase
mujacica Dec 3, 2025
53bcbc3
Cleanup the implementation after rebase
mujacica Dec 3, 2025
7aa1b4a
Simplify integration and enable feature-flag support
mujacica Nov 19, 2025
5ea3f3e
Fix typing
mujacica Nov 19, 2025
ebe9c30
feat(perforce): Add backend support for Perforce integration
mujacica Nov 11, 2025
57eab64
Finalize rebase
mujacica Nov 19, 2025
9dde7f8
feat(perforce): Implement repository/depot and code mapping logic
mujacica Nov 19, 2025
c51bb14
Fix PR comments
mujacica Nov 19, 2025
7b5d993
feat(perforce): Implement stacktrace linking and file blame (annotate…
mujacica Nov 19, 2025
5bdbe84
Fix PR comments, deduplicate logic
mujacica Nov 19, 2025
9c202a1
Adaptations after PR comments
mujacica Nov 21, 2025
75aaa74
Fix handling of file revisions for suspect commits
mujacica Nov 21, 2025
b671660
Fixes after rebase
mujacica Dec 3, 2025
cdc3cf4
Fix PR comments
mujacica Dec 3, 2025
023e6eb
Reworks after meeting with Perforce
mujacica Dec 4, 2025
d8a3079
Suspect commit fixes
mujacica Dec 4, 2025
91415ca
Finalize rebase
mujacica Dec 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 132 additions & 5 deletions src/sentry/integrations/perforce/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import logging
from collections.abc import Generator, Sequence
from contextlib import contextmanager
from datetime import datetime, timezone
from typing import Any, TypedDict

from P4 import P4, P4Exception
Expand All @@ -12,6 +13,7 @@
from sentry.integrations.services.integration import RpcIntegration, RpcOrganizationIntegration
from sentry.integrations.source_code_management.commit_context import (
CommitContextClient,
CommitInfo,
FileBlameInfo,
SourceLineInfo,
)
Expand Down Expand Up @@ -165,7 +167,7 @@ def _connect(self) -> Generator[P4]:

# Assert SSL trust after connection (if needed)
# This must be done after p4.connect() but before p4.run_login()
if self.ssl_fingerprint and self.p4port.startswith("ssl:"):
if self.ssl_fingerprint and self.p4port.startswith("ssl"):
try:
p4.run_trust("-i", self.ssl_fingerprint)
except P4Exception as trust_error:
Expand Down Expand Up @@ -357,6 +359,46 @@ def get_user(self, username: str) -> P4UserInfo | None:
# User not found - return None (not an error condition)
return None

def get_author_info_from_cache(
self, username: str, user_cache: dict[str, P4UserInfo | None]
) -> tuple[str, str]:
"""
Get author email and name from username with caching.

Args:
username: Perforce username
user_cache: Cache dictionary for user lookups

Returns:
Tuple of (author_email, author_name)
"""
author_email = f"{username}@perforce"
author_name = username

# Fetch user info if not in cache
if username not in user_cache:
try:
user_cache[username] = self.get_user(username)
except Exception as e:
logger.warning(
"perforce.get_author_info.user_lookup_failed",
extra={
"username": username,
"error": str(e),
"error_type": type(e).__name__,
},
)
user_cache[username] = None

user_info = user_cache.get(username)
if user_info:
if user_info.get("email"):
author_email = user_info["email"]
if user_info.get("full_name"):
author_name = user_info["full_name"]

return author_email, author_name

def get_changes(
self,
depot_path: str,
Expand Down Expand Up @@ -435,17 +477,102 @@ def get_blame_for_files(
self, files: Sequence[SourceLineInfo], extra: dict[str, Any]
) -> list[FileBlameInfo]:
"""
Get blame information for multiple files using p4 filelog.
Get blame information for multiple files using p4 changes.

Uses 'p4 filelog' + 'p4 describe' which is much faster than 'p4 annotate'.
Returns the most recent changelist that modified each file.
Uses 'p4 changes -m 1 -l' to get the most recent changelist that modified each file.
This is simpler and faster than using p4 filelog + p4 describe.

Note: This does not provide line-specific blame. It returns the most recent
changelist for the entire file, which is sufficient for suspect commit detection.

API docs:
- p4 changes: https://www.perforce.com/manuals/cmdref/Content/CmdRef/p4_changes.html

Returns a list of FileBlameInfo objects containing commit details for each file.

Performance notes:
- Makes ~2 P4 API calls per file: changes (with -l for description), user (cached)
- User lookups are cached within the request to minimize redundant calls
- Perforce doesn't have explicit rate limiting like GitHub
- Individual file failures are caught and logged without failing entire batch
"""
return []
blames: list[FileBlameInfo] = []
user_cache: dict[str, P4UserInfo | None] = {}

with self._connect() as p4:
for file in files:
try:
# Build depot path for the file (includes stream if specified)
# file.ref contains the stream but we are ignoring it since it's
# already part of the depot path we get from stacktrace (SourceLineInfo)
depot_path = self.build_depot_path(file.repo, file.path, None)

# Use p4 changes -m 1 -l to get most recent change for this file
# -m 1: limit to 1 result (most recent)
# -l: include full changelist description
changes = p4.run("changes", "-m", "1", "-l", depot_path)

if changes and len(changes) > 0:
change = changes[0]
changelist = change.get("change", "")
username = change.get("user", "unknown")

# Get author email and name with caching
author_email, author_name = self.get_author_info_from_cache(
username, user_cache
)

# Handle potentially null/invalid time field
time_value = change.get("time") or 0
try:
time_int = int(time_value)
except (TypeError, ValueError) as e:
logger.warning(
"perforce.client.get_blame_for_files.invalid_time_value",
extra={
**extra,
"changelist": changelist,
"time_value": time_value,
"error": str(e),
"repo_name": file.repo.name,
"file_path": file.path,
},
)
time_int = 0

commit = CommitInfo(
commitId=str(changelist),
committedDate=datetime.fromtimestamp(time_int, tz=timezone.utc),
commitMessage=change.get("desc", "").strip(),
commitAuthorName=author_name,
commitAuthorEmail=author_email,
)

blame_info = FileBlameInfo(
lineno=file.lineno,
path=file.path,
ref=file.ref,
repo=file.repo,
code_mapping=file.code_mapping,
commit=commit,
)
blames.append(blame_info)

except P4Exception as e:
# Log but don't fail for individual file errors
logger.warning(
"perforce.client.get_blame_for_files.error",
extra={
**extra,
"error": str(e),
"repo_name": file.repo.name,
"file_path": file.path,
"file_lineno": file.lineno,
},
)
continue

return blames

def get_file(
self, repo: Repository, path: str, ref: str | None, codeowners: bool = False
Expand Down
14 changes: 7 additions & 7 deletions src/sentry/integrations/perforce/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class PerforceMetadata(TypedDict, total=False):


DESCRIPTION = """
Connect your Sentry organization to your Perforce/Helix Core server to enable
Connect your Sentry organization to your P4 Core server to enable
stacktrace linking, commit tracking, suspect commit detection, and code ownership.
View source code directly from error stack traces and identify suspect commits that
may have introduced issues.
Expand All @@ -55,7 +55,7 @@ class PerforceMetadata(TypedDict, total=False):
FeatureDescription(
"""
Link your Sentry stack traces back to your Perforce depot files with support
for Helix Swarm web viewer. Automatically maps error locations to
for P4 Code Review viewer. Automatically maps error locations to
source code using configurable code mappings.
""",
IntegrationFeatures.STACKTRACE_LINK,
Expand Down Expand Up @@ -145,8 +145,8 @@ class PerforceInstallationForm(forms.Form):
required=False,
)
web_url = forms.URLField(
label=_("Helix Swarm URL (Optional)"),
help_text=_("Optional: URL to Helix Swarm web viewer for browsing files"),
label=_("P4 Code Review URL (Optional)"),
help_text=_("Optional: URL to P4 Code Review web viewer for browsing files"),
widget=forms.URLInput(attrs={"placeholder": "https://swarm.company.com"}),
required=False,
assume_scheme="https",
Expand All @@ -166,7 +166,7 @@ def clean_web_url(self) -> str:

class PerforceIntegration(RepositoryIntegration, CommitContextIntegration):
"""
Integration for Perforce/Helix Core version control system.
Integration for P4 Core version control system.
Provides stacktrace linking to depot files and suspect commit detection.
"""

Expand Down Expand Up @@ -471,9 +471,9 @@ def get_organization_config(self) -> list[dict[str, Any]]:
{
"name": "web_url",
"type": "string",
"label": "Helix Swarm URL (Optional)",
"label": "P4 Core URL (Optional)",
"placeholder": "https://swarm.company.com",
"help": "Optional: URL to Helix Swarm web viewer for browsing files",
"help": "Optional: URL to P4 Core web viewer for browsing files",
"required": False,
},
]
Expand Down
32 changes: 2 additions & 30 deletions src/sentry/integrations/perforce/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,37 +164,9 @@ def _extract_commit_info(
# Convert Unix timestamp to ISO 8601 format
timestamp = datetime.fromtimestamp(time_int, tz=timezone.utc).isoformat()

# Get user information from Perforce
# Get user information from Perforce using shared helper
username = change.get("user", "unknown")
author_email = f"{username}@perforce"
author_name = username

# Fetch user info if not in cache (skip "unknown" placeholder)
if username != "unknown" and username not in user_cache:
try:
user_cache[username] = client.get_user(username)
except Exception as e:
# Log user lookup failures but don't fail the entire commit processing
logger.warning(
"perforce.format_commits.user_lookup_failed",
extra={
"changelist": change.get("change"),
"username": username,
"error": str(e),
"error_type": type(e).__name__,
},
)
# Cache None to avoid repeated failed lookups for the same user
user_cache[username] = None

user_info = user_cache.get(username)
if user_info:
# Use actual email from Perforce if available
if user_info.get("email"):
author_email = user_info["email"]
# Use full name from Perforce if available
if user_info.get("full_name"):
author_name = user_info["full_name"]
author_email, author_name = client.get_author_info_from_cache(username, user_cache)

return P4CommitInfo(
id=str(change["change"]),
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/templates/sentry/integrations/perforce-config.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
{% block title %} {% trans "Perforce Setup" %} | {{ block.super }} {% endblock %}

{% block main %}
<h3>{% trans "Configure Perforce/Helix Core Connection" %}</h3>
<p>{% trans "Enter your Perforce server credentials to connect Sentry with your Perforce/Helix Core server." %}</p>
<h3>{% trans "Configure P4 Core Connection" %}</h3>
<p>{% trans "Enter your Perforce server credentials to connect Sentry with your P4 Core server." %}</p>
<p>{% trans "See the" %} <a href="https://docs.sentry.io/organization/integrations/source-code-mgmt/perforce/" target="_blank">{% trans "docs" %}</a> {% trans "for more information." %}</p>

<form action="" method="post" class="form-stacked">
Expand Down
Loading
Loading