Merge branch 'master' of github.com:bitwarden/contributing-docs into …

…ps/migrations

* 'master' of github.com:bitwarden/contributing-docs: (37 commits)
  chore(deps): update dependency cspell to v7.3.7 (#206)
  [PM-4161] Fix build command on README.md (#207)
  Fixed typo in csharp.md (#180)
  Update our EDD process documentation (#166)
  chore(deps): update actions/checkout action to v4.1.0 (#204)
  chore(deps): lock file maintenance (#205)
  fix(deps): update npm minor to v2.4.3 (#203)
  use dash when running self-hosted dotnet profile (#202)
  chore(deps): update actions/checkout action to v4 (#191)
  chore(deps): lock file maintenance (#201)
  chore(deps): update npm minor (#195)
  fix(deps): update dependency docusaurus-lunr-search to v3 (#200)
  setup_secrets: Pass `-clear` as switch (#194)
  Update KeyConnector docs for ARM Macs (#171)
  Lock file maintenance (#193)
  Update dependency cspell to v7.3.5 (#192)
  Update dependency ubuntu to v22 (#174)
  Update gh minor (#181)
  Feature flag documentation updates and mentions about new local override capabilities (#187)
  Update npm minor (#188)
  ...

# Conflicts:
#	custom-words.txt
#	docs/contributing/database-migrations/edd.mdx
#	docs/contributing/database-migrations/index.md

.github/CODEOWNERS

@@ -5,7 +5,7 @@
 # https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners
 
 # Leads for all reviews of documentation.
-* @bitwarden/team-leads-eng
+* @bitwarden/tech-leads
 
 # DevOps for Actions and other workflow changes.
 .github/workflows @bitwarden/dept-devops

.github/workflows/build.yml

@@ -15,13 +15,13 @@ defaults:
 jobs:
   lint:
     name: Build
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
     steps:
       - name: Checkout repo
-        uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
+        uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
 
       - name: Set up Node
-        uses: actions/setup-node@e33196f7422957bea03ed53f6fbb155025ffc7b8 # v3.7.0
+        uses: actions/setup-node@5e21ff4d9bc1a8cf6de233a3057d20ec6b3fb69d # v3.8.1
         with:
           cache: "npm"
           cache-dependency-path: "**/package-lock.json"

.github/workflows/lint.yml

@@ -15,13 +15,13 @@ defaults:
 jobs:
   lint:
     name: Lint
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
     steps:
       - name: Checkout repo
-        uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
+        uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
 
       - name: Set up Node
-        uses: actions/setup-node@e33196f7422957bea03ed53f6fbb155025ffc7b8 # v3.7.0
+        uses: actions/setup-node@5e21ff4d9bc1a8cf6de233a3057d20ec6b3fb69d # v3.8.1
         with:
           cache: "npm"
           cache-dependency-path: "**/package-lock.json"

README.md

@@ -23,7 +23,7 @@ reflected live without having to restart the server.
 ## Build
 
 ```bash
-npm build
+npm run build
 ```
 
 This command generates static content into the `build` directory and can be served using any static

custom-words.txt

@@ -1,6 +1,6 @@
-# Custom dictionary for spellchecking
-# Before adding a word here, consider whether you can put it in a single (`) or multiline (```) code snippet instead,
-# as they are automatically ignored by the spellchecker
+# Custom dictionary for spellchecking. Before adding a word here, consider whether you can put
+# it in a single (`) or multiline (```) code snippet instead, as they are automatically ignored
+# by the spellchecker. Please keep the list sorted alphabetically.
 
 Bitwarden
 bytemark
@@ -30,6 +30,7 @@ oktapreview
 Omnisharp
 onboarded
 opid
+OTLP
 passcode
 passwordless
 pinentry
@@ -41,6 +42,7 @@ roadmaps
 SCIM
 SDET
 SDLC
+Serilog
 signtool
 signup
 sprocs

docs/architecture/adr/0020-observability-with-opentelemetry.md

@@ -0,0 +1,121 @@
+---
+adr: "0020"
+status: In progress
+date: 2023-07-13
+tags: [server]
+---
+
+# 0020 - Observability with OpenTelemetry
+
+<AdrTable frontMatter={frontMatter}></AdrTable>
+
+## Context and Problem Statement
+
+Along with the maturation of the codebase over the years, the number of users on the platform has
+also grown significantly and more insight is needed into how services are performing at a
+fine-grained level. External profilers can certainly be attached in any running environment, but the
+platform itself needs to offer internal metrics not just to support self-hosted customers running
+the product but to enable engineers to improve it and tackle performance issues with solid data and
+evidence as to what and why something should change.
+
+## Considered Options
+
+:::note
+
+Bitwarden currently uses [Datadog][dd] as its monitoring tool and desires to increase its usage by
+engineers across the board to improve what we deliver.
+
+:::
+
+- **Maintain current observability options** - Expect those running the platform to configure what
+  they need outside of it for log collection and profiling / monitoring.
+- **Extend the platform to specifically support Datadog** - [Tracing for Datadog][ddtracer] exists
+  in package form and could be coded into application startup. Datadog-specific signals and metrics
+  can be collected via code and sent to the platform.
+- **Implement native instrumentation** - Add logic via what's available from
+  [`System.Diagnostics`][native] for custom instrumentation, and expect profiling to be configured
+  per the first option above.
+- **Use open observability standards** - Utilize [OpenTelemetry][otel] and emit signals on the
+  console as well as utilize its own eventing approach for instrumentation and metrics data.
+
+## Decision Outcome
+
+Chosen option: **Use open observability standards**.
+
+A strong alternative exists in just using native instrumentation, and not tying the platform to the
+implementation of any specific ecosystem -- even an open standard like OpenTelemetry. .NET closely
+supports OpenTelemetry metric collection integration but the desired power will be in how that data
+is used via output mechanisms like OTLP. A profiler attached to running components is independent of
+the availability of metrics via other means such as collection by an agent.
+
+Accessibility to metrics via configuration wins out over the expectation to set up and manage a
+profiler.
+
+### Positive Consequences
+
+- Console logging of metrics, if desired for use, fits well into container and orchestration tools,
+  and said environments can install agents for their collection.
+- No new dependencies that are merely aligned with the Bitwarden-specific cloud and its service
+  providers.
+- Components can be monitored with far more detail and lead to future improvements.
+- Use of an open standard like OpenTelemetry creates future flexibility for monitoring and
+  observability to grow with the expansion of that ecosystem, examples being the OTLP export vs.
+  just console logging.
+
+### Negative Consequences
+
+- Addition of the OpenTelemetry dependency across all services.
+- Proprietary profiler implementations may offer signal information that OpenTelemetry can't,
+  including automatic instrumentation.
+- With the capability to capture signals within the platform comes the burden of needing to maintain
+  clear policies around not capturing sensitive data.
+
+### Plan
+
+.NET Core's `System.Diagnostics` library supports the emission of metrics compatible with
+OpenTelemetry, and traces and metrics within the platform will become available on the console and
+via OTLP export. Configuration will be provided to turn either on or off with new application
+settings e.g.:
+
+```json
+{
+  "OpenTelemetry": {
+    "UseTracingExporter": "Console",
+    "UseMetricsExporter": "Console",
+    "Otlp": {
+      "Endpoint": "http://localhost:4318"
+    }
+  }
+}
+```
+
+Console and OTLP options will be available for the metrics and tracing export, along with the
+ability to specify a gRPC or HTTP endpoint for OTLP. Segmentation of activities will continue to be
+made using the configurable `ProjectName`.
+
+The initial implementation will provide default instrumentation details coming from ASP.NET Core and
+any used HTTP clients. Within Bitwarden the automatic instrumentation (profiler) may be explored at
+a future date but a code-first solution is desired to allow for more control and less setup during
+installation. It is expected that local processes will ingest logs / exports as desired.
+
+Software development lifecycle enhancements will be made to clarify best practices and review
+requirements for logging or monitoring changes. A [deep dive](/architecture/deep-dives) will be
+added on logging and monitoring to showcase patterns for adding signal collection in code. Only
+component runtime signals will be collected to start; no application payloads such as input and
+output data will be collected in signals.
+
+Over time and where needed, application logic to track custom [signals][otelsignals] (activities and
+meters) will be approached for deeper insights, especially in critical code paths. Standards will be
+developed and documented in the above deep dive on how to approach metric collection, without also
+collecting sensitive information. Core utility classes will be developed that establish a
+centralization of OpenTelemetry usage and make use in components easier and generic.
+
+Observability functionality will be moved to a new shared library -- separate from the core -- for
+host-oriented utilities. This library will be distributed as a NuGet package so that local `server`
+projects as well as new, independent repositories for services can receive the benefits.
+
+[dd]: https://www.datadoghq.com/
+[ddtracer]: https://www.nuget.org/packages/Datadog.Trace.Bundle
+[native]: https://learn.microsoft.com/en-us/dotnet/core/diagnostics/metrics-instrumentation
+[otel]: https://opentelemetry.io/
+[otelsignals]: https://opentelemetry.io/docs/concepts/signals/

docs/architecture/adr/0021-standard-output-logging.md

@@ -0,0 +1,131 @@
+---
+adr: "0021"
+status: In progress
+date: 2023-07-13
+tags: [server]
+---
+
+# 0021 - Logging to Standard Output
+
+<AdrTable frontMatter={frontMatter}></AdrTable>
+
+## Context and Problem Statement
+
+As the server platform has matured so have the various logging extensions to support additional use
+cases and customer requests. [Serilog][serilog] is in place via shared "core" logic for all
+services, initiated at startup, and over time additional "sinks" for specific use cases have been
+added with their needed configuration and downstream dependencies, increasing the size of the core
+library footprint.
+
+Maintenance needs have grown to keep sink dependencies up to date and more are desired to be added.
+Some of the presently-available sinks have very little use and / or better alternatives now exist.
+There is a growing list of conditions on how and when to use certain types of structured logging.
+Service-specific configuration, predicates, and filters are in place making it difficult to know
+what will be logged and when.
+
+## Considered Options
+
+:::note
+
+Bitwarden currently uses [Datadog][dd] as its monitoring tool and desires to increase its usage by
+engineers across the board to improve what we deliver.
+
+:::
+
+- **Maintain current logging options** - Support what is available today for logging methods and
+  expect those running the platform to configure what they need outside of it for log collection.
+- **Extend the platform to specifically support Datadog** - A Serilog sink [exists][ddsink] and the
+  platform can send logs directly to Datadog.
+- **Consolidate logging providers** - Announce deprecation and migration plans for sinks not aligned
+  with core needs and center on standard output for logs.
+
+## Decision Outcome
+
+Chosen option: **Consolidate logging providers**.
+
+Given long-term plans to adopt a more flexible shared (hosting extensions) library that can be used
+across services either as a project (server monolith) reference or NuGet package (and as a reference
+architecture), using Serilog as a way to extend native logging capabilities is beneficial. Details
+around how Serilog is implemented along with its advanced inputs and outputs can be extracted away
+into the shared library and driven at consuming applications via configuration.
+
+### Positive Consequences
+
+- Streamlined logging experience across components.
+- Standard output logging fits well into container and orchestration tools.
+- Elimination of several third-party dependencies and their maintenance, as well as global settings.
+- No new dependencies that are merely aligned with the Bitwarden-specific cloud and its service
+  providers.
+
+### Negative Consequences
+
+- A small number of users will need to migrate to standard output or similar ingestion of logs.
+- The Admin Portal log browsing function will leave (if configured) in favor of using whatever is
+  configured for log processing.
+
+### Plan
+
+Using standard support policies, release notes will include a mention that three Serilog sinks will
+be removed:
+
+- CosmosDb
+- Sentry
+- Syslog
+
+The remaining sinks -- core functionality of Serilog -- will continue to be supported:
+
+- Console
+- File
+
+While the Serilog [console sink][serilogconsole] is currently an implicit dependency with what's
+provided for ASP.NET Core, it will be explicitly referenced.
+
+Solutions exist for users to shift processing of logs for the removed sinks to standard output or
+file and retain their integration. Admin Portal users can similarly continue to use CosmosDb for log
+retention, but it is suggested that application monitoring that's available be used instead that
+should in essentially all cases be able to receive and process standard output logs.
+
+Cloud installations -- including Bitwarden's own -- will shift to configuration via environment
+variables or otherwise to utilize structured standard output logs for processing explicitly with
+[Serilog configuration][serilogconfig] e.g.:
+
+```json
+{
+  "Serilog": {
+    "Using": ["Serilog.Sinks.Console"],
+    "MinimumLevel": "Verbose",
+    "WriteTo": [{ "Name": "Console" }],
+    "Enrich": ["FromLogContext"],
+    "Properties": {
+      "Project": "BitwardenProjectName"
+    }
+  }
+}
+```
+
+This will allow better usage of `appsettings.json` and a richer developer experience. Existing
+built-in [.NET Core logging][netcorelogging] will continue to be available if desired, but the
+recommendation will be to move to a Serilog configuration.
+
+Code cleanup will be performed around Serilog usage, such as:
+
+- Removal of overuse of inclusion predicates that complicate (or sometimes block) effective log
+  output, for example in the uses of `AddSerilog` in place today at each consuming application.
+- Alignment with .NET Core and Serilog best practices on [initialization][seriloginit] and usage of
+  Serilog itself.
+- Improvements in logging initialization reliability and working with configuration issues, as well
+  as more resilient tear-down when a component stops / ends.
+- Removal of the above-deprecated sinks, in the final release of the support window.
+
+Logging functionality will be moved to a new shared library -- separate from the core project -- as
+mentioned above for host-oriented utilities. This library will be distributed as a NuGet package so
+that local `server` projects as well as new, independent repositories for services can receive the
+benefits.
+
+[serilog]: https://serilog.net/
+[dd]: https://www.datadoghq.com/
+[ddsink]: https://www.nuget.org/packages/serilog.sinks.datadog.logs
+[serilogconsole]: https://www.nuget.org/packages/serilog.sinks.console
+[serilogconfig]: https://www.nuget.org/packages/Serilog.Settings.Configuration/
+[netcorelogging]: https://learn.microsoft.com/en-us/dotnet/core/extensions/logging
+[seriloginit]: https://github.com/serilog/serilog-aspnetcore#two-stage-initialization

docs/contributing/code-style/csharp.md

@@ -51,7 +51,7 @@ this naming convention:
 ## Constructors
 
 - Multiple **constructors** should be separated by a newline (empty line between)
-- Constructors with multiple arguments should have arguments should be included 1 per line
+- Constructors with multiple arguments should have 1 argument listed per line
 - Empty constructors, when necessary, should be all 1-line, i.e., `public ClassName() { }`
 
 ## Control Blocks